[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 --- Comment #24 from Jonathan Wakely --- It's at https://gcc.gnu.org/wiki/VerboseDiagnostics#enum_switch
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 --- Comment #23 from Eric Gallager --- (In reply to Manuel López-Ibáñez from comment #8) > > Perhaps I should add an entry to the FAQ summarizing the above (anyone feel > free to beat me to it...) The "Commonly-reported Non-bugs" page would be another good place for it besides the FAQ, seeing as this keeps coming up: https://gcc.gnu.org/bugs/#nonbugs (searching for "enum" finds nothing on that page so far)
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 --- Comment #22 from Andrew Pinski --- *** Bug 93968 has been marked as a duplicate of this bug. ***
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 Andrew Pinski changed: What|Removed |Added CC||gcc at cookiesoft dot de --- Comment #21 from Andrew Pinski --- *** Bug 93967 has been marked as a duplicate of this bug. ***
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 Andrew Pinski changed: What|Removed |Added CC||yuri at tsoft dot com --- Comment #20 from Andrew Pinski --- *** Bug 93120 has been marked as a duplicate of this bug. ***
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 --- Comment #19 from DB --- just for anyone finding this later, note this has been raised at the level of the C++ Standard - http://wg21.link/p0375r0 - with the conclusion being: > EWG pointed out that most enumerations are exhaustive, and > we want to be annotating the exception, not the norm. > However, there wasn’t much appetite for a “non-exhaustive” > annotation, either; implementers were of the opinion that > current heuristics for diagnostics for switch statements > on enumerations are sufficient. - via https://botondballo.wordpress.com/2016/07/06/trip-report-c-standards-meeting-in-oulu-june-2016/ I suspect that "most enumerations are exhaustive" is an overgeneralisation that only holds for some people you could ask, but hey.
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479
--- Comment #18 from DB ---
(In reply to Jonathan Wakely from comment #17)
> (In reply to DB from comment #12)
> > (In reply to Jonathan Wakely from comment #11)
> > > Given enum E { E1 = 1, E3 = 3 } the values of the type are 0, 1, 2 and 3
> > > and
> > > -fstrict-enums tells the compiler it will never have a value outside that
> > > range. It does **not** tell it that the type will never have the value 0
> > > or
> > > 2.
> >
> > Huh. So allows non-named values and only enforces min/max, so doesn't
> > account for folk doing bitwise ORing?
>
> Huh? I have no idea what you mean by doesn't account for bitwide ORing. The
> fact you can have non-named values is essential for bitwide ORing.
>
> enum Bitmask { bit1 = 1, bit2 = 2, bit4 = 4 };
> Bitmask b = Bitmask(bit1|bit4);
>
> This creates a value that doesn't correspond to a named enumerator, but
> obviously this is valid. (For this type the values of the type are [0,7]
> because 7 is the highest value that can be represented in the minimum number
> of bits needed to represent all the enumerators).
My doubt ultimately arose from a momentary fail at bitwise arithmetic (thinking
1 | 3 == 4, how embarrassing), but I'm glad it elicited this confirmation!
Thanks.
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479
--- Comment #17 from Jonathan Wakely ---
(In reply to DB from comment #12)
> (In reply to Jonathan Wakely from comment #11)
> > Given enum E { E1 = 1, E3 = 3 } the values of the type are 0, 1, 2 and 3 and
> > -fstrict-enums tells the compiler it will never have a value outside that
> > range. It does **not** tell it that the type will never have the value 0 or
> > 2.
>
> Huh. So allows non-named values and only enforces min/max, so doesn't
> account for folk doing bitwise ORing?
Huh? I have no idea what you mean by doesn't account for bitwide ORing. The
fact you can have non-named values is essential for bitwide ORing.
enum Bitmask { bit1 = 1, bit2 = 2, bit4 = 4 };
Bitmask b = Bitmask(bit1|bit4);
This creates a value that doesn't correspond to a named enumerator, but
obviously this is valid. (For this type the values of the type are [0,7]
because 7 is the highest value that can be represented in the minimum number of
bits needed to represent all the enumerators).
That's how enumerated types work in C++, although many people seem to
misunderstand them. Enumerations are simply named constants, they don't affect
the type, or codegen, or what the valid values of the type are (except where
the values of the highest and lowest determine the range of valid values).
(In reply to Eric Gallager from comment #15)
> (In reply to Jonathan Wakely from comment #11)
> > (In reply to Eric Gallager from comment #6)
> > > This should probably depend on the -fstrict-enums flag, as that controls
> > > whether enums can have any value or just those values that are enumerated.
> >
> > No, that's not what it does.
> >
> > It tells the compiler the enum will only be one of the values of the
> > enumeration, which is not the same as the values corresponding to
> > enumerators.
> >
>
>
> That's a confusing distinction; they sound like the same thing at first to
> someone like me who doesn't speak standards-ese...
Well ... it's not.
> > As I said in comment 3, the OP's type has the values of int, because it has
> > an underlying type of int.
> >
> > Given enum E { E1 = 1, E3 = 3 } the values of the type are 0, 1, 2 and 3 and
> > -fstrict-enums tells the compiler it will never have a value outside that
> > range. It does **not** tell it that the type will never have the value 0 or
> > 2.
>
>
> Thanks, that example helps clear things up. Could it be added to the
> documentation for -fstrict-enums?
It already says "the values of the enumeration (as defined in the C++ standard;
basically, a value that can be represented in the minimum number of bits needed
to represent all the enumerators)."
The minimum number of bits needed to represent E1 and E3 is two bits, and both
0 and 2 can be represented in two bits.
Can I request that any further discussion happens elsewhere (the gcc-help list
or a general C++ programming forum) because it doesn't belong here.
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 --- Comment #16 from Jonathan Wakely --- (In reply to Eric Gallager from comment #14) > (In reply to Manuel López-Ibáñez from comment #9) > > In summary, neither adding 'default' or 'return' are recommended to silence > > this warning if you think the warning is wrong. If you think the warning > > will always be wrong, use __builtin_unreachable(). If you think it is wrong > > now, but you would like to notice if it stops being wrong, then use > > assert(false). > > > This is probably an issue for a separate bug, but speaking of > __builtin_unreachable(), now that GCC is going to start recognizing the > lint-style comment of: > /* FALLTHROUGH */ > for the benefit of -Wimplicit-fallthrough, could it also start recognizing > the lint-style comment of: > /* NOTREACHED */ > as a synonym for __builtin_unreachable()? I've seen comments like that in a > lot of code, actually, and it'd be a more portable solution than > __builtin_unreachable(). No No No No :-) Recognising comments like "FALLTHROUGH" and "fall-through" is a pragmatic solution to avoid introducing new false-positive warnings for old code that works as designed. (New code C++ should use [[attributes]] not comments, and C code can use GNU __attributes__, and [[attributes]] have been proposed for C too). Comments should not alter codegen, which __builtin_unreachable does. Also, NOTREACHED is IME just one of countless variations on comments such as "We should never get here" and "If we get here it's a bug" etc. and matching such arbitrary comments in infeasible.
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479
--- Comment #15 from Eric Gallager ---
(In reply to Jonathan Wakely from comment #11)
> (In reply to Eric Gallager from comment #6)
> > This should probably depend on the -fstrict-enums flag, as that controls
> > whether enums can have any value or just those values that are enumerated.
>
> No, that's not what it does.
>
> It tells the compiler the enum will only be one of the values of the
> enumeration, which is not the same as the values corresponding to
> enumerators.
>
That's a confusing distinction; they sound like the same thing at first to
someone like me who doesn't speak standards-ese...
>
> As I said in comment 3, the OP's type has the values of int, because it has
> an underlying type of int.
>
> Given enum E { E1 = 1, E3 = 3 } the values of the type are 0, 1, 2 and 3 and
> -fstrict-enums tells the compiler it will never have a value outside that
> range. It does **not** tell it that the type will never have the value 0 or
> 2.
Thanks, that example helps clear things up. Could it be added to the
documentation for -fstrict-enums?
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 --- Comment #14 from Eric Gallager --- (In reply to Manuel López-Ibáñez from comment #9) > In summary, neither adding 'default' or 'return' are recommended to silence > this warning if you think the warning is wrong. If you think the warning > will always be wrong, use __builtin_unreachable(). If you think it is wrong > now, but you would like to notice if it stops being wrong, then use > assert(false). This is probably an issue for a separate bug, but speaking of __builtin_unreachable(), now that GCC is going to start recognizing the lint-style comment of: /* FALLTHROUGH */ for the benefit of -Wimplicit-fallthrough, could it also start recognizing the lint-style comment of: /* NOTREACHED */ as a synonym for __builtin_unreachable()? I've seen comments like that in a lot of code, actually, and it'd be a more portable solution than __builtin_unreachable().
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 --- Comment #13 from Manuel López-Ibáñez --- (In reply to DB from comment #10) > Yeah, I've since thought of using abort(), which as you say, silences the > warning - and indicates with sufficient strength that this shouldn't happen. > assert() isn't much use since the warning would return as soon I compile in > release mode... been there, tried that. :) You can write your own "verify()" function that works like assert() but it will still be there in release mode. I think gnulib has something like that. The important bit is that it is declared noreturn. > I don't see why it gives "even more licence" to optimise, though, since it'd > be conditional per enum, rather than -fstrict-enums, which applies > globally... so if anyone inadvertently invokes UB, they've got much more > chance of being caught out if they're using the existing global option, than > if there was a per-enum attribute for whether or not the value must be a > named enumerator. For example, if you add a check that the enum is inside its range, the compiler with -fstrict-enums has every right to delete it, even if the enum is actually outside its range (due to a bug). And the compiler may not warn because it may not be able to prove that it is used outside its range at the point it gives the warning. Things only become more complex and unpredictable if each enum can be behave differently.
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479
--- Comment #12 from DB ---
(In reply to Jonathan Wakely from comment #11)
> Given enum E { E1 = 1, E3 = 3 } the values of the type are 0, 1, 2 and 3 and
> -fstrict-enums tells the compiler it will never have a value outside that
> range. It does **not** tell it that the type will never have the value 0 or
> 2.
Huh. So allows non-named values and only enforces min/max, so doesn't account
for folk doing bitwise ORing? Seems a little like the worst of both worlds. ;)
Thanks for the info anyway.
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479
--- Comment #11 from Jonathan Wakely ---
(In reply to Eric Gallager from comment #6)
> This should probably depend on the -fstrict-enums flag, as that controls
> whether enums can have any value or just those values that are enumerated.
No, that's not what it does.
It tells the compiler the enum will only be one of the values of the
enumeration, which is not the same as the values corresponding to enumerators.
As I said in comment 3, the OP's type has the values of int, because it has an
underlying type of int.
Given enum E { E1 = 1, E3 = 3 } the values of the type are 0, 1, 2 and 3 and
-fstrict-enums tells the compiler it will never have a value outside that
range. It does **not** tell it that the type will never have the value 0 or 2.
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 --- Comment #10 from DB --- (In reply to Manuel López-Ibáñez from comment #8) Thanks for the thoughts! > Those "artificial kludges" not only silence the warning, but also make the > code more readable and help the optimizer. A call to abort() or to any > noreturn function, for example, assert(), will also silence the warning and > it is safer than adding a new option to silence it. Yeah, I've since thought of using abort(), which as you say, silences the warning - and indicates with sufficient strength that this shouldn't happen. assert() isn't much use since the warning would return as soon I compile in release mode... been there, tried that. :) > You can use #pragma GCC diagnostic to disable the warning for this > particular function. If that seems even more of kludge than adding assert(), > you are right. But then, adding a #pragma around your whole codebase (which > is what a new option amounts to) sounds even worse, no?(In reply to DB from > comment #7) Thanks for the hint, and yeah, that would probably produce even messier code ;-) Good to know it exists though. > Perhaps there are uses for an __attribute__((strict_enum)) that can be > attached to particular enums (it would be a more fine-grained version of > -fstrict-enums). However, the effort to implement such a thing and keep it > working seems huge for and the potential fallout for handling it wrong (or > users misusing the attribute). Such an attribute is basically giving even > more license to the compiler to optimize based on undefined behavior, and > users often find UB incomprehensible. Compare that with the effort of > asking users to add __builtin_unreachable() (if they are sure their code > will never be wrong) or add an assert(false) (if they are not completely > sure). Yeah, I think this would be best as a Standard thing, rather than adding another custom attribute to any given compiler. I don't see why it gives "even more licence" to optimise, though, since it'd be conditional per enum, rather than -fstrict-enums, which applies globally... so if anyone inadvertently invokes UB, they've got much more chance of being caught out if they're using the existing global option, than if there was a per-enum attribute for whether or not the value must be a named enumerator. > Perhaps I should add an entry to the FAQ summarizing the above (anyone feel > free to beat me to it...) Couldn't hurt. :) There are some great explanations collected around the net (including some on IRC, but I'm sure you know those ;) but it's hard to find a pre-emptive summary of why this isn't already done. Thanks!
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 --- Comment #9 from Manuel López-Ibáñez --- In summary, neither adding 'default' or 'return' are recommended to silence this warning if you think the warning is wrong. If you think the warning will always be wrong, use __builtin_unreachable(). If you think it is wrong now, but you would like to notice if it stops being wrong, then use assert(false).
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 Manuel López-Ibáñez changed: What|Removed |Added CC||manu at gcc dot gnu.org --- Comment #8 from Manuel López-Ibáñez --- (In reply to DB from comment #5) > Of course a malicious and/or absent-minded caller could pass in a value that > the switch doesn't handle and invoke UB, but what about codebases that don't > let in such callers? Should they have to artificially add a > default/return/abort/whatever just to keep their build log readable? Do you mean codebases with zero bugs and that only omniscient programmers are allowed to edit and use? ;-) Those "artificial kludges" not only silence the warning, but also make the code more readable and help the optimizer. A call to abort() or to any noreturn function, for example, assert(), will also silence the warning and it is safer than adding a new option to silence it. > So, I feel GCC could do one better by - quite rightly! - defaulting to > warning about such callers - but, crucially, allowing the warning to be > disabled by competent ones (willing to take the blame if they pass a duff > value later) You can use #pragma GCC diagnostic to disable the warning for this particular function. If that seems even more of kludge than adding assert(), you are right. But then, adding a #pragma around your whole codebase (which is what a new option amounts to) sounds even worse, no?(In reply to DB from comment #7) > But it hits on what I'm going for: ensuring the compiler that I'll only use > named enumerator values. Ideally though, the Standard or compiler would > provide a way to specify that as an attribute per enum, so the programmer > could mix both schemes. I personally haven't used enums as bitmasks, so my > programs have only used discrete enumerators, but the bitfield case seems > allowed and common. Still, for cases where I'm not using it, I'd like a way > to tell the compiler to trust me! Perhaps there are uses for an __attribute__((strict_enum)) that can be attached to particular enums (it would be a more fine-grained version of -fstrict-enums). However, the effort to implement such a thing and keep it working seems huge for and the potential fallout for handling it wrong (or users misusing the attribute). Such an attribute is basically giving even more license to the compiler to optimize based on undefined behavior, and users often find UB incomprehensible. Compare that with the effort of asking users to add __builtin_unreachable() (if they are sure their code will never be wrong) or add an assert(false) (if they are not completely sure). Perhaps I should add an entry to the FAQ summarizing the above (anyone feel free to beat me to it...)
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 --- Comment #7 from DB --- Interesting switch, thanks - doesn't make any difference to warnings at the moment, though. But it hits on what I'm going for: ensuring the compiler that I'll only use named enumerator values. Ideally though, the Standard or compiler would provide a way to specify that as an attribute per enum, so the programmer could mix both schemes. I personally haven't used enums as bitmasks, so my programs have only used discrete enumerators, but the bitfield case seems allowed and common. Still, for cases where I'm not using it, I'd like a way to tell the compiler to trust me!
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 Eric Gallager changed: What|Removed |Added CC||egall at gwmail dot gwu.edu --- Comment #6 from Eric Gallager --- This should probably depend on the -fstrict-enums flag, as that controls whether enums can have any value or just those values that are enumerated.
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 DB changed: What|Removed |Added CC||db0451 at gmail dot com --- Comment #5 from DB --- Sorry to wake the dead, but I'm wondering whether anyone has ever considered adding an opt-out for this diagnostic, or whether there's any other way to issue the following hint to g++: > "Believe me: you will only ever get one of these values, so don't worry about > it" Of course a malicious and/or absent-minded caller could pass in a value that the switch doesn't handle and invoke UB, but what about codebases that don't let in such callers? Should they have to artificially add a default/return/abort/whatever just to keep their build log readable? Interestingly, Clang is silent if you handle all your enum (including non-class) cases (and if you don't, says "control **may** reach end of non-void function [-Wreturn-type]") - which, yeah, seems nice initially - but arguably is a worse default, due to the aforementioned malicious caller and what Jonathan explained. So, I feel GCC could do one better by - quite rightly! - defaulting to warning about such callers - but, crucially, allowing the warning to be disabled by competent ones (willing to take the blame if they pass a duff value later) Anyway, just some idle thoughts and questions. I understand the current reasoning and don't disagree with the default, but think there's a potential gain here too.
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 Paolo Carlini changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution||INVALID --- Comment #4 from Paolo Carlini 2012-10-10 19:11:20 UTC --- Closing.
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 --- Comment #3 from Jonathan Wakely 2012-05-24 20:21:57 UTC --- No, there's nothing wrong with the cast. A scoped enumeration type without an explicitly-specified underlying type has a fixed underlying type of int, so the values of the enumeration type are the values of int. Your switch doesn't handle all values, so control can flow off the end of the function, so the warning is correct.
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 --- Comment #2 from Georg Rudoy <0xd34df00d at gmail dot com> 2012-05-24 20:04:56 UTC --- (In reply to comment #1) > Foo f = Foo(2); > assert( DoFoo( f ) ); > > Undefined behaviour. Yes, it is. And isn't it happening at the point of cast of an integer to the enum class type?
[Bug c++/53479] Control flow analysis reports warnings in switch over an enum class even if all possible values have their branches
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53479 --- Comment #1 from Jonathan Wakely 2012-05-24 19:57:19 UTC --- Foo f = Foo(2); assert( DoFoo( f ) ); Undefined behaviour.
