Bug 61414 - enum class bitfield size-checking needs a separate warning flag controlling it
Summary: enum class bitfield size-checking needs a separate warning flag controlling it
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: unknown
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic, patch
Depends on:
Blocks: 44209
  Show dependency treegraph
 
Reported: 2014-06-04 17:30 UTC by Tom Tromey
Modified: 2019-04-25 21:27 UTC (History)
13 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-09-28 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Tromey 2014-06-04 17:30:28 UTC
Consider this source:

enum class K : unsigned int {
  V = 27
};

struct S {
  K v : 5;
};

When I compile this with Fedora 20 g++ (4.8.2-based) I get:

barimba. g++ --syntax-only -pedantic -std=c++11 /tmp/q.cc
/tmp/q.cc:6:9: warning: ‘S::v’ is too small to hold all values of ‘enum class K’ [enabled by default]
   K v : 5;
         ^

However, 5 bits is sufficient to hold all the values of K.
The warning goes away if I increase the size to 32, though of
course that defeats the purpose.
Comment 1 Tom Tromey 2014-06-04 17:44:06 UTC
Jonathan pointed out that this is not really a bug because
an enumeration with a fixed underlying type has a different
definition of its underlying values.

However, the bug still exists if the underlying type is not fixed:

enum class K {
  V = 27
};

struct S {
  K v : 6;
};

barimba. g++ --syntax-only -pedantic -std=c++11 /tmp/q.cc
/tmp/q.cc:6:9: warning: ‘S::v’ is too small to hold all values of ‘enum class K’ [enabled by default]
   K v : 6;
         ^
Comment 2 Daniel Krügler 2014-06-04 18:42:48 UTC
(In reply to Tom Tromey from comment #1)
> However, the bug still exists if the underlying type is not fixed:
> 
> enum class K {
>   V = 27
> };

This enum K also has a fixed underlying type (int). Contrary to unscoped enums, *all* scoped enums have a pre-defined underlying type that does not depend on the individual enumerators.
Comment 3 Tom Tromey 2014-06-04 18:46:07 UTC
(In reply to Daniel Krügler from comment #2)
> (In reply to Tom Tromey from comment #1)
> > However, the bug still exists if the underlying type is not fixed:
> > 
> > enum class K {
> >   V = 27
> > };
> 
> This enum K also has a fixed underlying type (int). Contrary to unscoped
> enums, *all* scoped enums have a pre-defined underlying type that does not
> depend on the individual enumerators.

Hah, shows what I know.

Regardless, this warning does not seem to be useful to me.
Comment 4 Pavel Revak 2014-12-11 21:27:19 UTC
I think that this warning is not on right place.

for example if I use bitfield:

struct S {
  unsigned int v : 5;
};

then this works without any warn,
so why I can't use enum class typed to unsigned int?
Comment 5 Xidorn Quan 2016-03-10 03:38:27 UTC
It seems G++ always throw that warning for enum class as bitfield, even when the enum class is actually empty:
> enum class K {};
> struct S {
>   K v : 5;
> };

G++ would give:
> test.cpp:3:9: warning: 'S::v' is too small to hold all values of 'enum class K'
>    K v : 5;
Comment 6 Jonathan Wakely 2016-09-28 23:04:28 UTC
(In reply to Xidorn Quan from comment #5)
> It seems G++ always throw that warning for enum class as bitfield, even when
> the enum class is actually empty:
> > enum class K {};
> > struct S {
> >   K v : 5;
> > };
> 
> G++ would give:
> > test.cpp:3:9: warning: 'S::v' is too small to hold all values of 'enum class K'
> >    K v : 5;

Yes, read comment 2.
Comment 7 Jonathan Wakely 2016-09-28 23:06:00 UTC
PR 51242 discusses the same issue, and PR 51242 comment 27 notes that Clang only warns when a value that isn't known to fit is assigned to the bitfield, which seems more useful.

At the very least there should be a -Wxxx switch controlling this option so it can be disabled.
Comment 8 Matt Godbolt 2016-10-27 19:34:19 UTC
Definitely a plus one to a switch to disable the warning: we currently have to work around this by putting code that does this in an -Isystem directory!
Comment 9 eric-bugs 2017-06-28 17:52:35 UTC
This does not seem like correct behavior to me either. The warning should be based on the maximum declared enum value, not the maximum possible value held by the underlying type.

After all as of C++17, the standard makes it undefined what happens if you try to stuff an integer into an enum value that doesn't correspond to one of the values listed in the enum declaration.
Comment 10 Tom Tromey 2017-07-24 13:26:29 UTC
I ran into this again, went to file a bug, and then found that
I'd already filed the bug...
Comment 11 Erik Rainey 2017-09-09 21:25:48 UTC
Present in 5.4 through 7.2 at least.
Comment 12 Jonathan Wakely 2017-10-16 00:05:05 UTC
(In reply to eric-bugs from comment #9)
> This does not seem like correct behavior to me either. The warning should be
> based on the maximum declared enum value, not the maximum possible value
> held by the underlying type.

No.

> After all as of C++17, the standard makes it undefined what happens if you
> try to stuff an integer into an enum value that doesn't correspond to one of
> the values listed in the enum declaration.

What? No it doesn't.
Comment 13 Sam van Kampen 2017-10-16 12:39:05 UTC
I've sent a patch that adds the warning flag -Wbitfield-enum-conversion to gcc-patches, and the gcc mailing list now also has a thread on the bug, and I thought I might share them on the bug tracker:

The patch:
https://gcc.gnu.org/ml/gcc-patches/2017-10/msg00989.html

The thread on the bug:
https://gcc.gnu.org/ml/gcc/2017-10/msg00121.html
Comment 14 Eric Gallager 2017-10-16 15:01:27 UTC
(In reply to Jonathan Wakely from comment #7)
> At the very least there should be a -Wxxx switch controlling this option so
> it can be disabled.

Agreed, that falls under bug 44209; retitling this one accordingly.

(In reply to Sam van Kampen from comment #13)
> I've sent a patch that adds the warning flag -Wbitfield-enum-conversion to
> gcc-patches, and the gcc mailing list now also has a thread on the bug, and
> I thought I might share them on the bug tracker:
> 
> The patch:
> https://gcc.gnu.org/ml/gcc-patches/2017-10/msg00989.html
> 
> The thread on the bug:
> https://gcc.gnu.org/ml/gcc/2017-10/msg00121.html

The name chosen for the flag makes it sound kinda related to bug 39170 but I guess the "enum" in the middle is enough of a differentiator.
Comment 15 Xavier B 2018-10-18 09:48:39 UTC
hi,

Just a ping for this issue.

Since it's about declarations and it is not possible to disable it,
as soon as you have headers files using the feature you get flooded with thousands of warnings making it impossible to see the other actually useful warnings...
Comment 16 Jonathan Wakely 2018-10-18 12:08:24 UTC
Sam, did you get a chance to implement the changes requested on the mailing list?
Comment 17 Barry Revzin 2019-04-25 21:27:20 UTC
Hi, it's me, another programmer who would love to use enum classes in bitfields instead of having to static_cast back and forth :-) Can anybody fixup Sam's patch?