Closed Bug 842886 Opened 11 years ago Closed 11 years ago

js/src/vm/Debugger.cpp:3013:32: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] and many more

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

mozilla-inbound now spams this pile of build warnings:
{
js/src/vm/Debugger.cpp: In member function ‘bool FlowGraphSummary::Entry::hasNoEdges() const’:
js/src/vm/Debugger.cpp:3013:32: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
js/src/vm/Debugger.cpp:3013:49: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
js/src/vm/Debugger.cpp: In member function ‘bool FlowGraphSummary::Entry::hasSingleEdge() const’:
js/src/vm/Debugger.cpp:3017:32: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
js/src/vm/Debugger.cpp:3017:49: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
js/src/vm/Debugger.cpp: In member function ‘bool FlowGraphSummary::Entry::hasMultipleEdgesFromSingleLine() const’:
js/src/vm/Debugger.cpp:3021:32: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
js/src/vm/Debugger.cpp:3021:49: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
js/src/vm/Debugger.cpp: In member function ‘bool FlowGraphSummary::Entry::hasMultipleEdgesFromMultipleLines() const’:
js/src/vm/Debugger.cpp:3025:32: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
js/src/vm/Debugger.cpp:3025:49: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
}

These are from https://hg.mozilla.org/integration/mozilla-inbound/rev/a02cad4f3238 (Bug 757188), which adds a bunch of "-1" comparisons & initializations for size_t variables (which are unsigned and hence can't take on the value -1).

We should be using SIZE_MAX instead of -1 there.  (Due to two's complement representation, the values end up being equivalent, but SIZE_MAX is more correct & won't make our compilers angry)
Attached patch fix v1Splinter Review
Attachment #715812 - Flags: review?
Attachment #715812 - Flags: review? → review?(ejpbruel)
Comment on attachment 715812 [details] [diff] [review]
fix v1

Review of attachment 715812 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for not catching this. Made the same mistake here as for the other patch. Will you land this for me? Otherwise I can do it tomorrow.

Thanks for filing a fix!
Attachment #715812 - Flags: review?(ejpbruel) → review+
No worries -- thanks for the quick review turnaround!

I added an "#include <limits.h>" at the top, too, for good measure (since that's what provides SIZE_MAX). Our JS code seems a bit conflicted on where to put standard-provided headers in the #include list, but from a quick sampling of .cpp files, it looked like the most popular option was "at the very top", so that's where I put it.

Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/9811bb0806fc
Assignee: general → dholbert
Status: NEW → ASSIGNED
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/9811bb0806fc
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: