The default bug view has changed. See this FAQ.

testIndexToString.cpp:46:5: warning: this decimal constant is unsigned only in ISO C90

RESOLVED FIXED in mozilla8

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

(Blocks: 1 bug)

Trunk
mozilla8
x86
Linux
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
Created attachment 553000 [details] [diff] [review]
fix: add 'u' suffix
Assignee: general → dholbert
Status: NEW → ASSIGNED
Attachment #553000 - Flags: review?(jwalden+bmo)
Why not do that with the others as well?
(Assignee)

Comment 3

6 years ago
'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. :))
Duplicate of this bug: 678780
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.
Attachment #553000 - Flags: review?(jwalden+bmo) → review+
(Assignee)

Comment 6

6 years ago
Landed as-is:
 http://hg.mozilla.org/integration/mozilla-inbound/rev/edd7aab15b8b
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/edd7aab15b8b
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.