All users were logged out of Bugzilla on October 13th, 2018

Fix self-comparison warnings in MemoryMetrics.h

RESOLVED FIXED in Firefox 42

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Waldo, Assigned: Waldo)

Tracking

unspecified
mozilla42
Points:
---

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
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

3 years ago
Created attachment 8636261 [details] [diff] [review]
Patch

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

3 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+
FYI, I have a patch locally which works by subtracting them and comparing against 0, instead of doing an equality test.
https://hg.mozilla.org/mozilla-central/rev/14b93413d8b8
Status: ASSIGNED → RESOLVED
Last Resolved: 3 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.