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

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

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

bergner at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950

            Bug ID: 87950
           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: ---

Noticed this for a while. 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. Maybe it's an intentional
disagreement over how to interpret the spec? AFAICT via objdump the generated
assembly is identical between clang & GCC even when compiled with
optimizations.

> 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;
}

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

> clang -Wall test.c
> gcc-8 --version
gcc-8 (Homebrew GCC 8.2.0) 8.2.0
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> clang --version
Apple LLVM version 10.0.0 (clang-1000.11.45.2)
Target: x86_64-apple-darwin17.7.0
Thread model: posix
InstalledDir:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

This applies to both C & C++.
Reply | Threaded
Open this post in threaded view
|

[Bug c/87950] GCC warns about reaching end of non-void function when all switch cases are completely handled

bergner at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
In C, enum valid value range is the full int range.
So all switch cases are not being handled even for valid C code.
Reply | Threaded
Open this post in threaded view
|

[Bug c/87950] GCC warns about reaching end of non-void function when all switch cases are completely handled

bergner at gcc dot gnu.org
In reply to this post by bergner at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950

--- Comment #2 from Vitali <vlovich at gmail dot com> ---
Why has clang made a different decision? Also, this warning is present in C++
code too.
Reply | Threaded
Open this post in threaded view
|

[Bug c/87950] GCC warns about reaching end of non-void function when all switch cases are completely handled

bergner at gcc dot gnu.org
In reply to this post by bergner at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950

Vitali <vlovich at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |UNCONFIRMED
         Resolution|INVALID                     |---

--- Comment #3 from Vitali <vlovich at gmail dot com> ---
Follow-up comment.
Reply | Threaded
Open this post in threaded view
|

[Bug c/87950] GCC warns about reaching end of non-void function when all switch cases are completely handled

bergner at gcc dot gnu.org
In reply to this post by bergner at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Vitali from comment #2)
> Why has clang made a different decision? Also, this warning is present in
> C++ code too.

You need to ask clang why they made that decision.  C rules are clear about
value ranges of enums and all.

In C++, (unlike C), out of ranges are just unspecified rather than undefined.
This means the warning is correct too.  NOTE there is a big difference between
the three: implmenentation defined, undefined and unspecified behaviors.
implmenentation defined is behavior that needs to be documented.  Undefined
behavior means the behavior can be different at different runs.  unspecified
behaviors means the behavior does not need to be documented but it has to be
constitiant between runs.

So there is no bug here again.

See PR12242 also about a warning for the unspecified behavior when doing the
conversion;  NOTE it is not undefined which is different.

See also https://gcc.gnu.org/ml/gcc-patches/2010-04/msg01694.html .

Also see PR 53479 for the C++ side of the warning too (for enum classes).

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Vitali from comment #2)
> Why has clang made a different decision? Also, this warning is present in
> C++ code too.

You need to ask clang why they made that decision.  C rules are clear about
value ranges of enums and all.

In C++, (unlike C), out of ranges are just unspecified rather than undefined.
This means the warning is correct too.  NOTE there is a big difference between
the three: implmenentation defined, undefined and unspecified behaviors.
implmenentation defined is behavior that needs to be documented.  Undefined
behavior means the behavior can be different at different runs.  unspecified
behaviors means the behavior does not need to be documented but it has to be
constitiant between runs.

So there is no bug here again.

See PR12242 also about a warning for the unspecified behavior when doing the
conversion;  NOTE it is not undefined which is different.

See also https://gcc.gnu.org/ml/gcc-patches/2010-04/msg01694.html .

Also see PR 53479 for the C++ side of the warning too (for enum classes).
Reply | Threaded
Open this post in threaded view
|

[Bug c/87950] GCC warns about reaching end of non-void function when all switch cases are completely handled

bergner at gcc dot gnu.org
In reply to this post by bergner at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Vitali from comment #2)
> Why has clang made a different decision? Also, this warning is present in
> C++ code too.

You need to ask clang why they made that decision.  C rules are clear about
value ranges of enums and all.

In C++, (unlike C), out of ranges are just unspecified rather than undefined.
This means the warning is correct too.  NOTE there is a big difference between
the three: implmenentation defined, undefined and unspecified behaviors.
implmenentation defined is behavior that needs to be documented.  Undefined
behavior means the behavior can be different at different runs.  unspecified
behaviors means the behavior does not need to be documented but it has to be
constitiant between runs.

So there is no bug here again.

See PR12242 also about a warning for the unspecified behavior when doing the
conversion;  NOTE it is not undefined which is different.

See also https://gcc.gnu.org/ml/gcc-patches/2010-04/msg01694.html .

Also see PR 53479 for the C++ side of the warning too (for enum classes).

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Vitali from comment #2)
> Why has clang made a different decision? Also, this warning is present in
> C++ code too.

You need to ask clang why they made that decision.  C rules are clear about
value ranges of enums and all.

In C++, (unlike C), out of ranges are just unspecified rather than undefined.
This means the warning is correct too.  NOTE there is a big difference between
the three: implmenentation defined, undefined and unspecified behaviors.
implmenentation defined is behavior that needs to be documented.  Undefined
behavior means the behavior can be different at different runs.  unspecified
behaviors means the behavior does not need to be documented but it has to be
constitiant between runs.

So there is no bug here again.

See PR12242 also about a warning for the unspecified behavior when doing the
conversion;  NOTE it is not undefined which is different.

See also https://gcc.gnu.org/ml/gcc-patches/2010-04/msg01694.html .

Also see PR 53479 for the C++ side of the warning too (for enum classes).
Reply | Threaded
Open this post in threaded view
|

[Bug c/87950] GCC warns about reaching end of non-void function when all switch cases are completely handled

bergner at gcc dot gnu.org
In reply to this post by bergner at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950

Vitali <vlovich at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |UNCONFIRMED
         Resolution|INVALID                     |---

--- Comment #6 from Vitali <vlovich at gmail dot com> ---
Actually as of C++17 it's undefined behaviour.

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

so at the very least when compiled with C++17 the compiler should be smart
enough to strengthen the control flow analysis.
Reply | Threaded
Open this post in threaded view
|

[Bug c/87950] GCC warns about reaching end of non-void function when all switch cases are completely handled

bergner at gcc dot gnu.org
In reply to this post by bergner at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Vitali from comment #6)
> Actually as of C++17 it's undefined behaviour.
>
> http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1766
>
> so at the very least when compiled with C++17 the compiler should be smart
> enough to strengthen the control flow analysis.

Open a C++ bug then rather than reusing a C bug report ...
Reply | Threaded
Open this post in threaded view
|

[Bug c/87950] GCC warns about reaching end of non-void function when all switch cases are completely handled

bergner at gcc dot gnu.org
In reply to this post by bergner at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950

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

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

--- Comment #8 from Martin Liška <marxin at gcc dot gnu.org> ---
Thank you Andrew for clarification of the behavior. Apparently it's quite
common question. One note that I have about C behavior is that we can maybe
also add -fstrict-enums to behave the same as in C++. Thoughts?
Reply | Threaded
Open this post in threaded view
|

[Bug c/87950] GCC warns about reaching end of non-void function when all switch cases are completely handled

bergner at gcc dot gnu.org
In reply to this post by bergner at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Martin Liška from comment #8)
> Thank you Andrew for clarification of the behavior. Apparently it's quite
> common question. One note that I have about C behavior is that we can maybe
> also add -fstrict-enums to behave the same as in C++. Thoughts?

No, we try not to add extensions which change well defined behavior to be
undefined behavior.  Adding -fstrict-enums for C would cause just that.
Reply | Threaded
Open this post in threaded view
|

[Bug c/87950] GCC warns about reaching end of non-void function when all switch cases are completely handled

bergner at gcc dot gnu.org
In reply to this post by bergner at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950

--- Comment #10 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #9)
> (In reply to Martin Liška from comment #8)
> > Thank you Andrew for clarification of the behavior. Apparently it's quite
> > common question. One note that I have about C behavior is that we can maybe
> > also add -fstrict-enums to behave the same as in C++. Thoughts?
>
> No, we try not to add extensions which change well defined behavior to be
> undefined behavior.  Adding -fstrict-enums for C would cause just that.

Good, then as mentioned explicit usage of __builtin_unreachable() will enable
desired optimization.

Then, let's close this as invalid?
Reply | Threaded
Open this post in threaded view
|

[Bug c/87950] GCC warns about reaching end of non-void function when all switch cases are completely handled

bergner at gcc dot gnu.org
In reply to this post by bergner at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
.
Reply | Threaded
Open this post in threaded view
|

[Bug c/87950] GCC warns about reaching end of non-void function when all switch cases are completely handled

bergner at gcc dot gnu.org
In reply to this post by bergner at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
             Status|RESOLVED                    |NEW
   Last reconfirmed|                            |2018-11-09
                 CC|                            |msebor at gcc dot gnu.org
         Resolution|INVALID                     |---
     Ever confirmed|0                           |1

--- Comment #12 from Martin Sebor <msebor at gcc dot gnu.org> ---
I do agree there is room for improvement here.  Consider the following test
case:

  $ cat t.c && gcc -O2 -S -Wall -Wextra -Wpedantic -Wswitch-enum
-fdump-tree-optimized=/dev/stdout t.c

  enum E { e0 } e;

  int f (void)
  {
    switch (e)
      case e0: return 0;
  }
  t.c: In function ‘f’:
  t.c:7:1: warning: control reaches end of non-void function [-Wreturn-type]
      7 | }
        | ^

  ;; Function f (f, funcdef_no=0, decl_uid=1909, cgraph_uid=1, symbol_order=1)

  f ()
  {
    E e.0_1;

    <bb 2> [local count: 1073741824]:
    e.0_1 = e;
    if (e.0_1 == 0)
      goto <bb 3>; [50.00%]
    else
      goto <bb 4>; [50.00%]

    <bb 3> [local count: 536870913]:
    return 0;

    <bb 4> [local count: 536870913]:
    return;

  }

The -Wswitch-enum option doesn't warn about the switch statement not handling
switch values that don't correspond to any of the enumerators.  To have it
diagnosed the -Wswitch-default option has to be explicitly specified (it's off
by default).

Given that, I believe the same logic should apply to the -Wreturn-type warning:
it should not trigger for the snippet above or for the test case in comment #0
unless explicitly requested somehow (perhaps it should consider
-Wswitch-default).

In addition, note the comparison to zero in the optimized GIMPLE.  It's
unnecessary because the function returns no value when the value of e is
non-zero.  Clang notices that and avoids the comparison.  But perhaps a better
example to illustrate the missed optimization opportunity than the contrived
test case above is this:

  enum E { e0, e1, e2 } e;

  int f (void)
  {
    switch (e)
    {
      case e0: return 1;
      case e1: return 2;
      case e2: return 3;
    }
  }

Here, GCC also emits the same pointless comparison/branch:

  f:
        movl    e(%rip), %eax
        cmpl    $2, %eax
        ja      .L2
        addl    $1, %eax
        ret
  .L2:
        ret