This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Default initialize enum flags to 0
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
This was my strongest argument, and I'm left without it, so...
> 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,
Pedro Alves