Closed
Bug 1185739
Opened 10 years ago
Closed 10 years ago
Fix self-comparison warnings in MemoryMetrics.h
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla42
| Tracking | Status | |
|---|---|---|
| firefox42 | --- | fixed |
People
(Reporter: Waldo, Assigned: Waldo)
References
Details
Attachments
(1 file)
|
2.26 KB,
patch
|
efaust
:
review+
|
Details | Diff | Splinter Review |
The Servo stuff in MemoryMetrics.h added a self-comparison -- that is, ServoSizes::Foo == ServoSizes::Foo for some enum initializer Foo. This triggers compiler self-comparison warnings up the wazoo with my clang.
| Assignee | ||
Comment 1•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8f5d0f09eec was the initial introduction of the warning. Key is in the macro, ServoSizes::servoKind is ServoSizes::GCHeapUsed, ServoSizes::MallocHeap, ServoSizes::NonHeap and so on from the FOR_EACH_SIZE macro, and comparing against ServoSizes::GCHeapUsed is a self-comparison. Work around this by doing a same-type check using the enum values (all >= 0) as array lengths. A bit arcane, but the comment explains why, at least.
Attachment #8636261 -
Flags: review?(dholbert)
Comment 2•10 years ago
|
||
Comment on attachment 8636261 [details] [diff] [review]
Patch
Review of attachment 8636261 [details] [diff] [review]:
-----------------------------------------------------------------
APPROVED.
::: js/public/MemoryMetrics.h
@@ +148,3 @@
> #define ADD_SIZE_TO_N(tabKind, servoKind, mSize) n += mSize;
> +#define ADD_SIZE_TO_N_IF_LIVE_GC_THING(tabKind, servoKind, mSize) \
> + /* Avoid self-comparison warnings by compare enums indirectly. */ \
nit: by comparing
Attachment #8636261 -
Flags: review?(dholbert) → review+
Comment 4•10 years ago
|
||
FYI, I have a patch locally which works by subtracting them and comparing against 0, instead of doing an equality test.
Comment 5•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in
before you can comment on or make changes to this bug.
Description
•