[PATCH] Fixing warnings revealed by gcc release build

[David Blaikie]

On Tue, Jan 29, 2013 at 11:30 AM, Edwin Vane <edwin.vane at intel.com> wrote:
>
>
> ================
> Comment at: lib/Basic/LangOptions.cpp:17-21
> @@ -16,3 +16,7 @@
>
> -const SanitizerOptions SanitizerOptions::Disabled = {};
> +const SanitizerOptions SanitizerOptions::Disabled = {
> +#define SANITIZER(NAME, ID) 0,
> +#include "clang/Basic/Sanitizers.def"
> +};
> +
>
> ----------------
> Dmitri Gribenko wrote:
>> Edwin Vane wrote:
>> > Dmitri Gribenko wrote:
>> > > Edwin Vane wrote:
>> > > > Dmitri Gribenko wrote:
>> > > > > David Blaikie wrote:
>> > > > > > Not sure of the motivation for this change - shouldn't the {} in the original code produce the same effect (zero initializing all the elements)?
>> > > > > I have mixed feelings about this.  -Wmissing-field-initializers is a different thing: all members are initialized by {}, but gcc complains that initializers are not explicitly spelled in the source.
>> > > > Is it a problem to explicitly do the initialization to make gcc happy if it has the same result as {}? I wasn't aware empty braces was defined to cause all fields to be initialized to 0.
>> > > It just adds noise to satisfy the warning.  In this particular case it is not actually that bad, since SanitizerOptions is generated by the same .def file, so it will not go out of sync.
>> > >
>> > Would you prefer using default constructor instead? At least I was able to find defined behaviour for this situation:
>> >
>> > const SanitizerOptions SanitizerOptions::Disabled = SanitizerOptions();
>> This is dynamic initialization, if I'm reading [basic.init.start] correctly.  (But Clang does it as static initialization because it is allowed to do so, but not required.)  #define/#include is better.
> Ok. So apparently {} generating warnings in gcc is a bug (http://stackoverflow.com/a/1539162).

OK

> That doesn't change the fact that the warning still exists and is annoying.

The fact that the warning fires is annoying, yes. I'll suggest that
most people use Clang to develop & keep -Werror on - we're certainly
clang -Werror clean & maintain that invariant with zeal.

> Using -Wno-missing-field-initializers seems heavy handed

Why is that heavy handed? If the warning is bad there's no reason to
hold us to it.

> and the #define/#include solution is short and relatively future-proof.

The point is that this is just one case of this warning - and
especially of a warning that shows up in GCC (ie: is seen by a
minority of developers because most of us develop with Clang) is
likely to regress often. If it's not a valuable warning, rather than
causing that noise - we should just disable it now & save us the fuss
forever more.

- David