[PATCH] Default initialize enum flags to 0
Simon Marchi
simon.marchi@polymtl.ca
Tue Feb 21 16:51:00 GMT 2017
On 2017-02-21 06:16, Pedro Alves wrote:
> On 02/21/2017 03:01 AM, Simon Marchi wrote:
>
>>
>>> #2 - The other reason is that it's nice IMO to leave enums and enum
>>> flags
>>> easily interchangeable -- i.e., make them behave as close as
>>> possible.
>>> Having one be default initialized, and the other value initialized
>>> means that when changing variables from one type to the other
>>> one needs to consider that aspect.
>>
>> Well, they're not directly interchangeable in C++, which is the whole
>> point of having enum flags.
>
> TBC, by "interchangeable" I meant, when you refactor/redesign code and
> decide the flags would be better as normal enums, and vice versa.
>
> Passing an enum flags to a function expecting a raw enum
> (because it was compiled in C) and vice versa would probably
> not be interchangeable at run time, depending on ABI.
>
>>> #3 - Default initializing to zero can hide bugs that would otherwise
>>> be caught with -Winitialized.
>>
>> (-Wuninitialized?)
>>
>> I don't really understand how this could hide a bug.
>
> I was thinking of the "this code path should have set flags to
> something
> non-zero, but the compiler didn't warn because the variable
> was initialized" kind of bug.
>
>> When we don't
>> initialize the field in the default constructor, does -Wuninitialized
>> issue a warning for this?
>>
>> my_flags flags;
>> flags |= some_flag;
>>
>> I tried quickly and it doesn't seem so. As stated above, if we have
>> the
>> default constructor of the enum flag initialize the value to 0, it
>> won't
>> be a bug in C++, but it will generate a warning in C where plain enums
>> are used.
>
> Bah, I assumed it did! But now that I try, it really doesn't. :-(
>
> I filed a GCC bug now:
>
> [-Wuninitialized] referencing uninitialized field of POD struct should
> warn
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79658
Oh so it's only in some very specific cases that the warning is
missing...
> This was my strongest argument, and I'm left without it, so...
TBH, my original motivation was so we could be lazy and leave out the
initializations, I only realized while reading your message that it was
actually a problem :).
>> So if we don't initialize the value to 0 in the default constructor,
>> compiling this code in C++ will be a bug but will not generate any
>> warning. This seems very error prone to me.
>
> Agreed, unfortunately...
>
> Looking at the patch:
>
>> @@ -117,6 +117,7 @@ private:
>> public:
>> /* Allow default construction, just like raw enums. */
>> enum_flags ()
>> + : m_enum_value ((enum_type) 0)
>> {}
>>
>
> The "just like raw enums" comment is no longer true. Please tweak
> that.
>
> OK with that fixed.
Thanks, pushed with that fixed.
More information about the Gdb-patches
mailing list