On Wed, Oct 18, 2017 at 3:15 PM, Sam van Kampen via gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
> On Wed, Oct 18, 2017 at 09:46:08AM -0600, Martin Sebor wrote:
>> > Fair enough, I didn't know whether to change the way it currently was
>> > triggered. Do you think it should fall under -Wextra (I don't think it
>> > falls under -Wall, since it isn't "easy to avoid or modify to prevent
>> > the warning" because it may be valid and wanted behavior), or should it
>> > be enabled by no other flag?
>>
>> I think it depends on the implementation of the warning. With
>> the current (fairly restrictive) behavior I'd say it should be
>> disabled by default. But if it were to be changed to more closely
>> match the Clang behavior and only warn for bit-field declarations
>> that cannot represent all enumerators of the enumerated type, then
>> including it in -Wall would seem helpful to me.
>>
>> I.e., Clang doesn't warn on this and IIUC that's what the reporter
>> of the bug also expects:
>>
>> enum E: unsigned { E3 = 15 };
>>
>> struct S { E i: 4; };
>>
>> (There is value in warning on this as well, but I think most users
>> will not be interested in it, so making the warning a two-level one
>> where level 1 warns same as Clang and level 2 same as GCC does now
>> might give us the best of both worlds).
>
> I see what you mean - that is the behavior I wanted to implement in the
> first place, but Jonathan Wakely rightly pointed out that when an
> enumeration is scoped, all values of its underlying type are valid
> enumeration values, and so the bit-field you declare in 'S' _is_ too
> small to hold all values of 'enum E'.
>
> Here's the corresponding text from draft N4659 of the C++17 standard,
> ยง10.2/8 [dcl.enum]
>
> For an enumeration whose underlying type is fixed, the values of the
> enumeration are the values of the underlying type. [...] It is possible
> to define an enumeration that has values not defined by any of its
> enumerators.
>
> Still, warning when a bit-field can't hold all enumerators instead of
> all values may be a good idea. I've looked into it, and it does require
> recalculating the maximum and minimum enumerator value, since the bounds
> of the underlying type are saved in TYPE_MIN_VALUE and TYPE_MAX_VALUE
> when the enumeration is scoped, instead of the min/max enumerator value.
>
> Would adding that separate warning level be part of a separate patch, or
> should I add it to this one?
I think making that behavior the default would be appropriate. The
current behavior for scoped enums seems like a bug; sure, all values
of the underlying type are valid, but that's also true for "unsigned i
: 4", and we don't warn about that.
Jason
