924608 - tautological compare warning in MemoryMetrics.h

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andrew McCreight [:mccr8]]

I get this in Clang for every file that includes that header:

 5:27.13 Warning: -Wtautological-compare in /home/amccreight/cent/obj-dbg/dist/include/js/MemoryMetrics.h: self-comparison always evaluates to true
 5:27.13 ../../../dist/include/js/MemoryMetrics.h:388:1140: warning: self-comparison always evaluates to true [-Wtautological-compare]
 5:27.13         n += (js::IsLiveGCThing == js::IsLiveGCThing) ? objectsGCHeapOrdinary : 0; n += (js::IsLiveGCThing == js::IsLiveGCThing) ? objectsGCHeapFunction : 0; [and it goes on and on]

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: use boolean-valued enums to avoid the tauto warning

Terrence suggested this fix. Basically, you just make the enum boolean valued.  Clang seems okay with boolean-constant tautologies.

Comment 2

[Terrence Cole [:terrence]]

Comment on attachment 814551 [details] [diff] [review]
use boolean-valued enums to avoid the tauto warning

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

https://tbpl.mozilla.org/?tree=Try&rev=7b30d8e1af26

::: js/public/MemoryMetrics.h
@@ +60,5 @@
>  #define DECL_SIZE(gc, mSize)                      size_t mSize;
>  #define ZERO_SIZE(gc, mSize)                      mSize(0),
>  #define COPY_OTHER_SIZE(gc, mSize)                mSize(other.mSize),
>  #define ADD_OTHER_SIZE(gc, mSize)                 mSize += other.mSize;
> +#define ADD_SIZE_TO_N_IF_LIVE_GC_THING(gc, mSize) n += gc ? mSize : 0;

Keep parens around the (gc) because MACROS.

Comment 3

[Andrew McCreight [:mccr8]]

Thanks Terrence!

Comment 4

[Andrew McCreight [:mccr8]]

Terrence, is this ready to land?  I can do a Windows try push if that's what's holding this back.

Comment 5

[Terrence Cole [:terrence]]

The try push I did above came back with windows orange and red, including some rebuilds later in the day. The errors look spurious, but when I checked the base's push on m-i the windows builds were green. Maybe it was temporary tbpl bustage? A second try run would not be amiss.

Comment 6

[Andrew McCreight [:mccr8]]

https://tbpl.mozilla.org/?tree=Try&rev=0ea88194b4f9

Comment 7

[Andrew McCreight [:mccr8]]

Windows try run is totally green.

Comment 8

[Terrence Cole [:terrence]]

Must have been my tip then. Pushed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c3d14caa839a

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c3d14caa839a