Last Comment Bug 737754 - nsScriptLoader.cpp:1286:50: warning: comparison of unsigned expression >= 0 is always true
: nsScriptLoader.cpp:1286:50: warning: comparison of unsigned expression >= 0 i...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Daniel Holbert [:dholbert]
:
Mentors:
Depends on:
Blocks: buildwarning 597368
  Show dependency treegraph
 
Reported: 2012-03-20 21:59 PDT by Daniel Holbert [:dholbert]
Modified: 2012-03-22 06:34 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix v1 (1.52 KB, patch)
2012-03-20 22:54 PDT, Daniel Holbert [:dholbert]
hsivonen: review+
Details | Diff | Review

Description Daniel Holbert [:dholbert] 2012-03-20 21:59:07 PDT
Filing bug on this build warning:
nsScriptLoader.cpp: In member function ‘nsresult nsScriptLoader::PrepareLoadedRequest(nsScriptLoadRequest*, nsIStreamLoader*, nsresult, PRUint32, const PRUint8*)’:
nsScriptLoader.cpp:1286:50: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
nsScriptLoader.cpp:1286:91: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
nsScriptLoader.cpp:1286:157: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
nsScriptLoader.cpp:1286:197: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]

This is for this chunk:
1286   NS_ASSERTION(mDeferRequests.IndexOf(aRequest) >= 0 ||
1287                mAsyncRequests.IndexOf(aRequest) >= 0 ||
1288                mNonAsyncExternalScriptInsertedRequests.IndexOf(aRequest) >= 0 ||
1289                mXSLTRequests.IndexOf(aRequest) >= 0 ||
1290                mPreloads.Contains(aRequest, PreloadRequestComparator()) ||
1291                mParserBlockingRequest,
1292                "aRequest should be pending!");

Those IndexOf functions all return unsigned values now, as of this change:
http://hg.mozilla.org/mozilla-central/diff/aacd84d91b66/content/base/src/nsScriptLoader.h
which switched those arrays from nsCOMArray (which returns a signed value from IndexOf) to nsTArray (which returns an unsigned value from IndexOf)

I think we need to change these IndexOf() calls to Contain() calls.
Comment 1 Daniel Holbert [:dholbert] 2012-03-20 22:54:22 PDT
Created attachment 607864 [details] [diff] [review]
fix v1
Comment 2 Mozilla RelEng Bot 2012-03-21 06:57:09 PDT
Autoland Patchset:
	Patches: 607864
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=2c6b4f398985
Try run started, revision 2c6b4f398985. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=2c6b4f398985
Comment 3 Daniel Holbert [:dholbert] 2012-03-21 11:02:31 PDT
Darn, looks like [autoland-try] just triggers builds & no tests.  I wanted to give this a unittest sanity-check, since this effectively re-enables an assertion that's been nerfed for a little while, and it's possible that we've since added a testcase (or some code) that fails the non-nerfed assertion.

I just pushed to try manually, so as not to confuse autoland with another request while it's waiting for the first run's results:
   https://tbpl.mozilla.org/?tree=Try&rev=160eaa422e12
Comment 4 Nathan Froyd [:froydnj] 2012-03-21 11:05:29 PDT
(In reply to Daniel Holbert [:dholbert] from comment #3)
> Darn, looks like [autoland-try] just triggers builds & no tests.

FWIW, you can instruct it to trigger tests with normal try syntax:

[autoland-try:-b do -p all -u all -t none]
Comment 5 Daniel Holbert [:dholbert] 2012-03-21 11:11:16 PDT
Yup, I know -- I'd just mistakenly assumed that [autoland-try] would trigger all unit tests (instead of no unit tests).
Comment 6 Nathan Froyd [:froydnj] 2012-03-21 11:15:14 PDT
Ah, yeah, it used to and then the behavior flipped a couple of weeks ago.
Comment 7 Mozilla RelEng Bot 2012-03-21 12:05:03 PDT
Try run for 2c6b4f398985 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=2c6b4f398985
Results (out of 14 total builds):
    success: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-2c6b4f398985
Comment 8 Daniel Holbert [:dholbert] 2012-03-21 14:07:14 PDT
Landed:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/377a8d6327aa
Comment 9 Marco Bonardo [::mak] 2012-03-22 06:34:50 PDT
https://hg.mozilla.org/mozilla-central/rev/377a8d6327aa

Note You need to log in before you can comment on or make changes to this bug.