[Bug c++/87951] New: GCC warns about reaching end of non-void function when all switch is completely handled

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] New: GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

            Bug ID: 87951
           Summary: GCC warns about reaching end of non-void function when
                    all switch is completely handled
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vlovich at gmail dot com
  Target Milestone: ---

If a function has a single switch statement that handles all enum values &
returns a value GCC will warn about the function not returning a value whereas
clang does not.  GCC requires an explicit __builtin_unreachable() annotation
after the switch. As of C++17 it seems to now be undefined behaviour rather
than unspecified behaviour for an enum to have a value that's not enumerated.

http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1766

> cat test.c

enum Enum {
  A,
  B,
};

int CoverMyBases(enum Enum x) {
        switch (x) {
                case A:
                        return 1;
                case B:
                        return 0;
        }
}

int main(int argc, const char **argv) {
        CoverMyBases(A);
        CoverMyBases(B);
        return 0;
}

> g++-8 -Wall --std=c++17 test.c

test.c: In function 'CoverMyBases':
test.c:16:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^

> clang++ -std=c++17 -Wall test.c

NOTE: Clang never warns about this even prior to C++17 or in C either.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2018-11-9
                 CC|                            |jason at gcc dot gnu.org,
                   |                            |marxin at gcc dot gnu.org,
                   |                            |redi at gcc dot gnu.org
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=87950

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
>
> http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1766
>

Can you please Jason and Jonathan comment about it? Can we strengthen behavior?
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
One more comment here. I do cooperate with our openSUSE maintainer of Chromium
package and they have quite some of these warnings when building with GCC. I
told him he can strengthen behavior with -fstrict-enums option. Once we did
that the package crashes during build.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Vitali from comment #0)
> If a function has a single switch statement that handles all enum values &
> returns a value GCC will warn about the function not returning a value
> whereas clang does not.  GCC requires an explicit __builtin_unreachable()
> annotation after the switch. As of C++17 it seems to now be undefined
> behaviour rather than unspecified behaviour for an enum to have a value
> that's not enumerated.

No, that's not what the defect report says. I wish this myth would die.

Outside the range of the enumerated values does not mean not enumerated.
Given enum E { e0=0, e10=7 } there is nothing undefined about E(1) or E(4).

> http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1766
>
> > cat test.c
>
> enum Enum {
>   A,
>   B,
> };
>
> int CoverMyBases(enum Enum x) {
> switch (x) {
> case A:
> return 1;
> case B:
> return 0;
> }
> }
>
> int main(int argc, const char **argv) {
> CoverMyBases(A);
> CoverMyBases(B);
> return 0;
> }
>
> > g++-8 -Wall --std=c++17 test.c
>
> test.c: In function 'CoverMyBases':
> test.c:16:1: warning: control reaches end of non-void function
> [-Wreturn-type]
>  }
>  ^

This is what -fstrict-enums is for.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

--- Comment #4 from Vitali <vlovich at gmail dot com> ---
Is there a way to annotate a specific enum as strict?
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

--- Comment #5 from Vitali <vlovich at gmail dot com> ---
Jonathan, I think the defect report here does actually apply to this example. I
agree the argument could be made that if there's gaps in the enum values that
it's arguable that the current GCC behaviour is standards compliant (clearly
clang & GCC disagree on this for both C & C++ so unclear who's right or wrong
in their interpretation of what's allowed).

However, in the example posted this is a "dense" enum. There's no integer value
possible that's not outside the range & yet GCC still continues to treat that
as a possibility & thus missing optimization opportunities & generating
false-positive warnings.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Vitali from comment #5)
> Jonathan, I think the defect report here does actually apply to this
> example.

I didn't say otherwise.

> I agree the argument could be made that if there's gaps in the enum
> values that it's arguable that the current GCC behaviour is standards
> compliant (clearly clang & GCC disagree on this for both C & C++ so unclear
> who's right or wrong in their interpretation of what's allowed).

No, it has nothing to do with "gaps" in the enum. This is a myth.

All that matters is the range of representable values, which (for an
enumeration without a fixed underlying type) depends on the number of bits
required to represent the minimum and maximum enumerator values. It doesn't
make any difference whatsoever whether there are "gaps" between those minimum
and maximum
values.

> However, in the example posted this is a "dense" enum.

What matters is that all the values of the enum can be represented in a single
bit, and so the only valid values are 0 and 1, which happen to have
enumerators. But the only valid values would still be 0 and 1 if your enum was
defined as
enum Enum { B=1 }; and so a switch that failed to handle the possibility of
Enum(0) would be wrong (even with -fstrict-enums)

The mental model of "dense" vs "gaps" is WRONG.

> There's no integer
> value possible that's not outside the range & yet GCC still continues to
> treat that as a possibility & thus missing optimization opportunities &
> generating false-positive warnings.

Yes, that's what -fstrict-enums is for.

The default (without -fstrict-enums) assumes most code is buggy and doesn't
follow the rules. If you do not use values outside the valid range of values
for the enumeration, use -fstrict-enums.

I can see some value in the suggestion to annotate a specific enumeration type
as strict, but that might not be the right solution. What matters is how the
enumeration type is used (whether invalid values are ever created) and that
isn't something you can guarantee when the enum is declared.

You *can* annotate the uses of a specific enum, but telling the compiler that
no other values will ever be used in the switch, by adding:

  default:
    __builtin_unreachable();
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

Askar Safin <safinaskar at mail dot ru> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |safinaskar at mail dot ru

--- Comment #7 from Askar Safin <safinaskar at mail dot ru> ---
"g++ -fstrict-enums" doesn't disable warning if I use "enum class" instead of
plain enum.

This is test code:
///
enum class Enum {
  A,
  B,
};

int CoverMyBases(Enum x) {
    switch (x) {
        case Enum::A:
            return 1;
        case Enum::B:
            return 0;
    }
}

int main(int argc, const char **argv) {
    CoverMyBases(Enum::A);
    CoverMyBases(Enum::B);
    return 0;
}
///
This is command line:
g++ -fstrict-enums a.cpp
This is gcc version: gcc version 8.2.0 (Debian 8.2.0-10)
And I get this:
a.cpp: In function ‘int CoverMyBases(Enum)’:
a.cpp:13:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Askar Safin from comment #7)
> "g++ -fstrict-enums" doesn't disable warning if I use "enum class" instead
> of plain enum.

Yes because they have different semantics ...
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

--- Comment #9 from Askar Safin <safinaskar at mail dot ru> ---
(In reply to Andrew Pinski from comment #8)
> Yes because they have different semantics ...

So, you mean that "enum class" is less strict than normal enums? This is very
strange.

Today I normally use "enum class", because they are advertised as "better". And
now I see that, well, they are *less* strict, than normal enums, and thus (to
get better compiler warnings) I need to switch to "old bad" enums, right?! This
is very-very strange. So if I want good compiler messages I need to convert my
code from "good modern" enum class to "bad old" enums, right?! Is there some
hack to get better compiler warnings for "enum class"? Something like
-fstrict-enums, but for "enum class"?
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
No.

They're not "less strict", but they have a fixed underlying type. For any
enumeration type with a fixed underlying type (whether "enum class" or just
"enum") the validvalues of the type are all the valid values of the underlying
type.

So:

enum E : unsigned char { e1, e2 };
enum class F : unsigned char { f1, f2 };

Both of these types can have any value from 0-255, so a switch statement that
only handles the e1/e2 or or f1/f2 is incorrect.

I wish people would just learn how enums work, it's not that complicated.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

--- Comment #11 from Askar Safin <safinaskar at mail dot ru> ---
(In reply to Jonathan Wakely from comment #10)
> I wish people would just learn how enums work, it's not that complicated.

Okey, now I understand everything. Now I see that, well, -fstrict-enums
silences warning for code from comment #1 only because number of enum
alternatives happen to be power of 2. If it would be, say, 3, then code from
comment #1 will always produce a warning even with -fstrict-enums. And also I
understand that code with "enum class" will always produce a warning.

So, gcc doesn't have way to enable clang-style warning behavior for enums. And
gcc doesn't have such way for "enum class", too. And -fstrict-enums enables
clang-style warning behavior for code from comment #1 only because this code
has power of 2 number of alternatives.

And also I understand that current gcc behavior is not bug and is absolutely
standard-compliant.

But, well, I still don't like current g++ behavior. I want g++ to not report
"reaching end of non-void..." if all switch alternatives are handled, at least
if some optional option is passed to gcc. I want clang behavior. Okey, you may
mark this bug as "wishlist", but I still think that clang-style behavior here
is very useful feature.

Look here, LLVM project was forced to introduce special kludge to make sure g++
will not give useless warnings:
http://llvm.org/docs/CodingStandards.html#don-t-use-default-labels-in-fully-covered-switches-over-enumerations
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

--- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It's not a useless warning. If I call your function from comment 7 like this, I
get undefined behaviour:

  CoverMyBases( Enum{2} );

Your switch is undefined for this code. That's what GCC is warning you about.
To tell GCC you will never call the function with any value except Enum::A or
Enum::B, add

  default: __builtin_unreachable();
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #12)

> It's not a useless warning. If I call your function from comment 7 like
> this, I get undefined behaviour:
>
>   CoverMyBases( Enum{2} );
>
> Your switch is undefined for this code. That's what GCC is warning you
> about. To tell GCC you will never call the function with any value except
> Enum::A or Enum::B, add
>
>   default: __builtin_unreachable();

Maybe better to add it after the switch if you want to make sure that you get a
warning with -Wswitch if a new enumerator is added.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
                 CC|                            |msebor at gcc dot gnu.org
         Resolution|---                         |DUPLICATE

--- Comment #14 from Martin Sebor <msebor at gcc dot gnu.org> ---
This is a duplicate of bug 87950, with most of the same discussion.

*** This bug has been marked as a duplicate of bug 87950 ***
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

Eric Gallager <egallager at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |egallager at gcc dot gnu.org
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=61864

--- Comment #15 from Eric Gallager <egallager at gcc dot gnu.org> ---
(In reply to Askar Safin from comment #11)

> (In reply to Jonathan Wakely from comment #10)
> > I wish people would just learn how enums work, it's not that complicated.
>
> Okey, now I understand everything. Now I see that, well, -fstrict-enums
> silences warning for code from comment #1 only because number of enum
> alternatives happen to be power of 2. If it would be, say, 3, then code from
> comment #1 will always produce a warning even with -fstrict-enums. And also
> I understand that code with "enum class" will always produce a warning.
>
> So, gcc doesn't have way to enable clang-style warning behavior for enums.
> And gcc doesn't have such way for "enum class", too. And -fstrict-enums
> enables clang-style warning behavior for code from comment #1 only because
> this code has power of 2 number of alternatives.
>
> And also I understand that current gcc behavior is not bug and is absolutely
> standard-compliant.
>
> But, well, I still don't like current g++ behavior. I want g++ to not report
> "reaching end of non-void..." if all switch alternatives are handled, at
> least if some optional option is passed to gcc. I want clang behavior. Okey,
> you may mark this bug as "wishlist", but I still think that clang-style
> behavior here is very useful feature.
>
> Look here, LLVM project was forced to introduce special kludge to make sure
> g++ will not give useless warnings:
> http://llvm.org/docs/CodingStandards.html#don-t-use-default-labels-in-fully-
> covered-switches-over-enumerations

For the warning clang uses mentioned in the link (-Wcovered-switch-default) see
bug 61864. (It looks like the behavior to respect unreachables has been updated
since the last time I checked it)
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

--- Comment #16 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Martin Sebor from comment #14)
> This is a duplicate of bug 87950, with most of the same discussion.
>
> *** This bug has been marked as a duplicate of bug 87950 ***

I think I need to add an "Enums 101" section to https://gcc.gnu.org/wiki/FAQ or
https://gcc.gnu.org/wiki/VerboseDiagnostics
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

--- Comment #17 from Vitali <vlovich at gmail dot com> ---
I was explicitly asked to open this as a separate bug in comment #7 of 87950.
Would be helpful if the GCC devs could coordinate to figure out if they want
separate bugs for C/C++ or 1 bug.

Jonathan, on this forum your feedback comes off a bit like Skinner where he
blames the children. There's clearly a gap between how compiler authors define
switches as working & how developers wish to use those enums/wish those enums
were actually defined. I appreciate there's a large amount of legacy code that
complicates this issue but I don't think that condescending to people trying to
communicate the issue in good faith moves the conversation along.

I'd argue that clang strikes a much better balance here in terms of the
practical implications of the warning even if it's not perfect. In my mind,
warnings are most useful when they optimize for a low rate of false positives.
The GCC warnings for enums do not in practice have a low rate of false
positives and are clearly tuned to minimize false negatives (i.e. accidentally
missing a switch statement that doesn't have all enums covered).

At the very least it would be useful to give users the power to turn on a
warning equivalent to Clang's that minimizes false negatives (or change
-Wswitch to be equivalent to Clang & add a -Wpedantic-switch that's part of
-pedantic for the current behaviour).

You could even provide annotations that let users annotate enums/switches in an
explicit fashion to indicate that they can be constructed with non-label
values/aren't expected to be depending on which flag you choose to use to
silence warnings/let the compiler optimize more fully.

In practice, definitely in C++, enums ARE intended to be a closed set of all
possible paths & the flags/random integer value cases are far less frequently
used; even for bitmasks it's a lazy way to do it. In C++ I usually define
strongly-typed types that represent the bitmask & overload the bit math
operators for the enum to yield that secondary type. I think considering that's
a technique used by Boost & Qt, it's reasonable to assume that having the enum
double as the storage type for the bitmask is more of a hack technically
allowed than one that is considered best practice by large C++ projects. In C
it's more muddled because there's no operator overloading, but the number of
enums that exist far outnumber the uses where it's used for bitwise math.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/87951] GCC warns about reaching end of non-void function when all switch is completely handled

asolokha at gmx dot com
In reply to this post by asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87951

Eric Gallager <egallager at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=81665

--- Comment #18 from Eric Gallager <egallager at gcc dot gnu.org> ---
(In reply to Vitali from comment #17)

> I was explicitly asked to open this as a separate bug in comment #7 of
> 87950. Would be helpful if the GCC devs could coordinate to figure out if
> they want separate bugs for C/C++ or 1 bug.
>
> Jonathan, on this forum your feedback comes off a bit like Skinner where he
> blames the children. There's clearly a gap between how compiler authors
> define switches as working & how developers wish to use those enums/wish
> those enums were actually defined. I appreciate there's a large amount of
> legacy code that complicates this issue but I don't think that condescending
> to people trying to communicate the issue in good faith moves the
> conversation along.
>
> I'd argue that clang strikes a much better balance here in terms of the
> practical implications of the warning even if it's not perfect. In my mind,
> warnings are most useful when they optimize for a low rate of false
> positives. The GCC warnings for enums do not in practice have a low rate of
> false positives and are clearly tuned to minimize false negatives (i.e.
> accidentally missing a switch statement that doesn't have all enums
> covered).
>
> At the very least it would be useful to give users the power to turn on a
> warning equivalent to Clang's that minimizes false negatives (or change
> -Wswitch to be equivalent to Clang & add a -Wpedantic-switch that's part of
> -pedantic for the current behaviour).
>
> You could even provide annotations that let users annotate enums/switches in
> an explicit fashion to indicate that they can be constructed with non-label
> values/aren't expected to be depending on which flag you choose to use to
> silence warnings/let the compiler optimize more fully.
>
> In practice, definitely in C++, enums ARE intended to be a closed set of all
> possible paths & the flags/random integer value cases are far less
> frequently used; even for bitmasks it's a lazy way to do it. In C++ I
> usually define strongly-typed types that represent the bitmask & overload
> the bit math operators for the enum to yield that secondary type. I think
> considering that's a technique used by Boost & Qt, it's reasonable to assume
> that having the enum double as the storage type for the bitmask is more of a
> hack technically allowed than one that is considered best practice by large
> C++ projects. In C it's more muddled because there's no operator
> overloading, but the number of enums that exist far outnumber the uses where
> it's used for bitwise math.

This reminded me of bug 81665