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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Daniel Holbert [:dholbert]]

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)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix v1

Comment 2

[Eddy Bruel [: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!

Comment 3

[Daniel Holbert [:dholbert]]

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

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9811bb0806fc