Last Comment Bug 678845 - testIndexToString.cpp:46:5: warning: this decimal constant is unsigned only in ISO C90
: testIndexToString.cpp:46:5: warning: this decimal constant is unsigned only i...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: mozilla8
Assigned To: Daniel Holbert [:dholbert]
:
Mentors:
: 678780 (view as bug list)
Depends on:
Blocks: buildwarning
  Show dependency treegraph
 
Reported: 2011-08-14 11:29 PDT by Daniel Holbert [:dholbert]
Modified: 2011-08-16 03:18 PDT (History)
2 users (show)
Ms2ger: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix: add 'u' suffix (1.18 KB, patch)
2011-08-14 11:34 PDT, Daniel Holbert [:dholbert]
jwalden+bmo: review+
Details | Diff | Review

Description Daniel Holbert [:dholbert] 2011-08-14 11:29:10 PDT
Filing bug on this GCC build warnings:
{
js/src/jsapi-tests/testIndexToString.cpp:46:5: warning: this decimal constant is unsigned only in ISO C90
js/src/jsapi-tests/testIndexToString.cpp:47:5: warning: this decimal constant is unsigned only in ISO C90
js/src/jsapi-tests/testIndexToString.cpp:48:5: warning: this decimal constant is unsigned only in ISO C90
js/src/jsapi-tests/testIndexToString.cpp:49:5: warning: this decimal constant is unsigned only in ISO C90
}

This is from this chunk:
> static const struct TestPair {
>     uint32 num;
>     const char *expected;
> } tests[] = {
[----------SNIP----------]
>     { 2147483648, "2147483648" },
>     { 2147483649, "2147483649" },
>     { 4294967294, "4294967294" },
>     { 4294967295, "4294967295" },
> };

The compiler is complaining about those integer literals - like all integer literals, they'll be treated as signed values in the compiled code -- but in this case, they're larger than INT_MAX, so they'll end up being represented with negative signed values.

Since we're trying to store them in a uint32 variable, we presumably want to consider them unsigned.  We can address this easily by just adding a "u" suffix and taking the implicit signed-int representation out of the loop.
Comment 1 Daniel Holbert [:dholbert] 2011-08-14 11:34:05 PDT
Created attachment 553000 [details] [diff] [review]
fix: add 'u' suffix
Comment 2 :Ms2ger 2011-08-14 11:35:26 PDT
Why not do that with the others as well?
Comment 3 Daniel Holbert [:dholbert] 2011-08-14 11:40:11 PDT
'cause it's not necessary? If Waldo prefers, I'd be happy to make that change.

(Of course, it's technically "not necessary" with the ones touched by the patch, too -- it's only necessary to avoid buildspam. :))
Comment 4 Phil Ringnalda (:philor) 2011-08-14 15:51:01 PDT
*** Bug 678780 has been marked as a duplicate of this bug. ***
Comment 5 Jeff Walden [:Waldo] (remove +bmo to email) 2011-08-15 08:52:10 PDT
Comment on attachment 553000 [details] [diff] [review]
fix: add 'u' suffix

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

My policy on tests is that it has to be clear what's happening, and the formatting/naming/etc. can't be misleading.  But beyond that I don't care about nitpickeries of absolute consistency, adherence to style guidelines, and so on.  :-)  So this is fine, and to be honest even if I did have reason to care about style I'm not sure I'd find this wrong.
Comment 6 Daniel Holbert [:dholbert] 2011-08-15 11:47:50 PDT
Landed as-is:
 http://hg.mozilla.org/integration/mozilla-inbound/rev/edd7aab15b8b

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