Closed
Bug 1114444
Opened 10 years ago
Closed 10 years ago
Fix clang warnings in js/src/gc/Statistics.cpp
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
INVALID
People
(Reporter: cpeterson, Assigned: cpeterson)
References
Details
Attachments
(1 file, 1 obsolete file)
2.20 KB,
patch
|
sfink
:
review+
Waldo
:
review+
|
Details | Diff | Splinter Review |
These are new compiler warnings in the diagnostics code added for bug 1103957:
js/src/gc/Statistics.cpp:889:24: warning: format specifies type 'long' but the argument has type 'int64_t' (aka 'long long') [-Wformat]
phase, phaseStartTimes[phase]);
^~~~~~~~~~~~~~~~~~~~~~
js/src/gc/Statistics.cpp:890:27: warning: comparison of integers of different signs: 'int' and 'size_t' (aka 'unsigned long') [-Wsign-compare]
for (int i = 0; i < phaseNestingDepth; i++)
~ ^ ~~~~~~~~~~~~~~~~~
Attachment #8539921 -
Flags: review?(sphink)
Comment 1•10 years ago
|
||
Comment on attachment 8539921 [details] [diff] [review]
fix-Statistics-warnings.patch
If the argument has a <stdint.h> type, the right thing to use is the macros in #include "mozilla/IntegerPrintfMacros.h". "%lld" is not guaranteed to match int64_t.
%zu isn't supported by MSVC. I see someone sub silentio added PRIuSIZE to IntegerPrintfMacros.h for this. But this violates the intent of the header: that its supported interface be *only* a subset of <inttypes.h>, with the goal to swap that in when bug-free <inttypes.h> is provided by all supported platforms. So don't use that, either. (I guess I need to file a bug to remove it.) For now just cast the fprintf argument to uintptr_t and use PRIuPTR til we figure out something to provide an alternative to %z.
Attachment #8539921 -
Flags: review?(sphink)
Assignee | ||
Comment 2•10 years ago
|
||
Patch v2: fprintf uint64_t with PRId64 and size_t with PRIuPTR.
Green Try build:
https://tbpl.mozilla.org/?tree=Try&rev=3f6f46f0f0ae
Attachment #8539921 -
Attachment is obsolete: true
Attachment #8540306 -
Flags: review?(jwalden+bmo)
Comment 3•10 years ago
|
||
Comment on attachment 8540306 [details] [diff] [review]
fix-Statistics-warnings-v2.patch
Review of attachment 8540306 [details] [diff] [review]:
-----------------------------------------------------------------
Gah. More to back out when these diagnostics bear fruit. Sorry about the warnings.
Attachment #8540306 -
Flags: review?(jwalden+bmo) → review+
Updated•10 years ago
|
Attachment #8540306 -
Flags: review+
Comment 4•10 years ago
|
||
Note that I have a fix for the actual problem now, so the code containing the warnings will be removed once that's r+. So It's up to you whether you want to push this in the meantime.
Comment 5•10 years ago
|
||
I pushed the other fix, so this code is gone.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•