1125371 - Silence some GCC-only warnings about uninitialized variables when building with --enable-optimize

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

There's no really satisfying fix for this kind of thing. My solution is to initialize the stupid variables, redundantly, iff the compiler is GCC.

Resisted the urge to call the macro MOZ_SHUT_UP_WESLEY().

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: Silence some GCC-only warnings about uninitialized varaibles when building with --enable-optimize

Comment 2

[Luke Wagner [:luke]]

Thanks for doing this.  FWIW, none of the initializations in src/asmjs should affect perf so I'd be happy if you just wanted to initialize them always (which is a little less noisy-looking).  Alternatively, if you want to record that a variable's init value is just being used to silence warnings (so some wise guy doesn't come along later and remove it), how about:
  int i = MOZ_SILENCE_UNDEF_WARNING(0);
which is a bit more compact.

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8553968 [details] [diff] [review]
Silence some GCC-only warnings about uninitialized varaibles when building with --enable-optimize

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

(thanks for doing this)

::: js/src/jit/LiveRangeAllocator.cpp
@@ +60,5 @@
>        case MUST_REUSE_INPUT:
>          n = JS_snprintf(cursor, end - cursor, "v%u", virtualRegister());
>          break;
> +      default:
> +        MOZ_CRASH("impossible Requirement::Kind");

the switch over the enum treats all existing cases. It'd be nice to just initialize n to -1 above, so that if we ever add a new value to this enum, the compiler complains that it isn't handled in this switch statement.

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 8553968 [details] [diff] [review]
Silence some GCC-only warnings about uninitialized varaibles when building with --enable-optimize

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

Thanks. Looks good, although I agree with Luke that always initializing seems simpler and shouldn't affect performance.

r=me either way.

Comment 5

[Jason Orendorff [:jorendorff]]

(In reply to Luke Wagner [:luke] from comment #2)
> Thanks for doing this.  FWIW, none of the initializations in src/asmjs
> should affect perf so I'd be happy if you just wanted to initialize them
> always (which is a little less noisy-looking).

The reason I did it that way is so that other, more cautious compilers would have a chance to tell us if the uninitialized variable is ever used. But I guess we can do without that.

>   int i = MOZ_SILENCE_UNDEF_WARNING(0);

But it'd have to be more like

    int i MOZ_SILENCE_UNDEF_WARNING(= 0);

which makes my eyes hurt.

Plain old initialization it is. I added comments saying stuff like:

    uint32_t newLength = 0;  // initialize to silence GCC warning

Compiler weirdness causes junk to appear in your code, episode 182...

Comment 6

[Luke Wagner [:luke]]

Oh, I didn't explain my idea well, I mean that:
  #define MOZ_SILENCE_UNDEF_WARNING(i) i
so that the macro was the comment, but I suppose the comment works just as well and less macros is better macros.

Comment 7

[Jason Orendorff [:jorendorff]]

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

Comment 8

[Carsten Book [:Tomcat]]

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