[Bug c/61864] New: Feature Request, -Wcovered-switch-default to identify "dead" default branch

[Bug c/61864] New: Feature Request, -Wcovered-switch-default to identify "dead" default branch

[chengniansun at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61864

            Bug ID: 61864
           Summary: Feature Request, -Wcovered-switch-default to identify
                    "dead" default branch
           Product: gcc
           Version: 4.10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: chengniansun at gmail dot com

Currently, GCC supports -Wswitch and -Wswitch-enum. But it does not warn on the
case that all the possible values of an enum are covered in a switch, making
the default branch unreachable. 

The following code is extracted and simplified from the SPEC benchmark, the
enum only has three values, which are all covered in the switch. Therefore the
default branch is "dead". 


$: cat t.c
enum clust_strategy {
  CLUSTER_MEAN, CLUSTER_MAX, CLUSTER_MIN
};

int Cluster(enum clust_strategy mode) {
  switch(mode) {
  case CLUSTER_MEAN: break;
  case CLUSTER_MAX: break;
  case CLUSTER_MIN: break;
  default: break;
  }
  return 0;
}
$: clang-trunk -Wcovered-switch-default -c t.c
t.c:10:3: warning: default label in switch which covers all enumeration values
[-Wcovered-switch-default]
  default: break;
  ^
1 warning generated.
$:

[Bug c/61864] Feature Request, -Wcovered-switch-default to identify "dead" default branch

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61864

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-07-22
                 CC|                            |mpolacek at gcc dot gnu.org
   Target Milestone|---                         |4.10.0
     Ever confirmed|0                           |1

--- Comment #1 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Confirmed.

[Bug c/61864] Feature Request, -Wcovered-switch-default to identify "dead" default branch

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61864

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.0                         |5.2

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 5.1 has been released.

[Bug c/61864] Feature Request, -Wcovered-switch-default to identify "dead" default branch

[egall at gwmail dot gwu.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61864

--- Comment #4 from Eric Gallager <egall at gwmail dot gwu.edu> ---
Actually, after giving this some more thought, and reading the GCC
documentation some more, I came up with some ideas for a compromise that could
allow both -Wswitch-default and -Wcovered-switch-default to be used without
conflicting:

* Seeing as -Wcovered-switch-default (as clang implements it) makes certain
assumptions about the nature of enums, its implementation in GCC should be
restricted to only being active when the -fstrict-enums flag is also in use. As
-fstrict-enums is a C++-only flag, this would effectively make
-Wcovered-switch-default a C++-only warning, which would allow C-only
programmers (such as myself) to avoid the flag.
* Besides general philosophical issues with the -Wcovered-switch-default flag,
there's also two specific implementation issues I have with clang's
implementation of it that I would want to see GCC fix in its own
implementation. I wrote a quick source file to demonstrate:

$ cat switches.c
#include <stdlib.h> /* for rand() */
int main(void)
{
    int swindex0 = rand();
    enum values {
        FIRST_ENUMERATOR,
        SECOND_ENUMERATOR,
        THIRD_ENUMERATOR
    } swindex1 = SECOND_ENUMERATOR; /* silence unitialized warnings */
    /* do the non-enumerator switch first: */
    switch (swindex0) {
        case 1:
            swindex1 = FIRST_ENUMERATOR;
            break;
        case 2:
            swindex1 = SECOND_ENUMERATOR;
            break;
        case 3:
            swindex1 = THIRD_ENUMERATOR;
            break;
        /* -Wswitch-default: usual warning about switch with no default */
    }
    if (swindex0 > 3) {
        swindex1 = THIRD_ENUMERATOR;
    } else if (swindex0 < 1) {
        swindex1 = FIRST_ENUMERATOR;
    }

    switch (swindex1) {
        case FIRST_ENUMERATOR:
            break;
        case SECOND_ENUMERATOR:
            break;
        case THIRD_ENUMERATOR:
            break;
        /* -Wcovered-switch-default: print a warning here like clang's,
         * but only if also used with -fstrict-enums: */
        default:
            break;
    }
    switch (swindex1) {
        case FIRST_ENUMERATOR:
            break;
        case SECOND_ENUMERATOR:
            break;
        case THIRD_ENUMERATOR:
            break;
        /* -Wswitch-default: still warns:
         * * if by itself, the usual message, as in the first switch.
         * * if with -Wcovered-switch-default, use a different message,
         *   like:
         *   "Warning: switch missing default case which would be covered if it
had one"
         *   "Note: use -fstrict-enums to silence this warning"
         * * if with -fstrict-enums, do like mentioned in the previous,
         *   and print no warning. */
    }
    switch (swindex1) {
        case FIRST_ENUMERATOR:
            break;
        case SECOND_ENUMERATOR:
            break;
        case THIRD_ENUMERATOR:
            break;
        /* Ideally there should be no warning here whatsoever; clang still
         * warns about it though: */
        default: __builtin_unreachable(); /*NOTREACHED*/
    }
    switch (swindex1) {
        case FIRST_ENUMERATOR:
            break;
        case SECOND_ENUMERATOR:
            break;
        case THIRD_ENUMERATOR: /*FALLTHROUGH*/
        /* Ideally there should be no warning from -Wcovered-switch-default
         * here, because even though there are labels for all enum values,
         * the 3rd case still falls through to the default case: */
        default: /*REACHEDBYFALLTHROUGH*/
            break;
        /* (clang still warns here though) */
    }
}

$ clang --version && clang -c -Weverything switches.c
clang version 3.6.0 (tags/RELEASE_360/final)
Target: x86_64-apple-darwin10.8.0
Thread model: posix
switches.c:38:9: warning: default label in switch which covers all enumeration
values [-Wcovered-switch-default]
        default:
        ^
switches.c:66:9: warning: default label in switch which covers all enumeration
values [-Wcovered-switch-default]
        default: __builtin_unreachable(); /*NOTREACHED*/
        ^
switches.c:77:9: warning: default label in switch which covers all enumeration
values [-Wcovered-switch-default]
        default: /*REACHEDBYFALLTHROUGH*/
        ^
3 warnings generated.


$ g++-6_git --version && g++-6_git -c -fstrict-enums -Wall -Wextra -Wswitch
-Wswitch-default -Wswitch-enum switches.c
g++-6_git (GCC) 6.0.0 20150604 (experimental)
Copyright (C) 2015 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.

switches.c: In function 'int main()':
switches.c:11:12: warning: switch missing default case [-Wswitch-default]
     switch (swindex0) {
            ^
switches.c:41:12: warning: switch missing default case [-Wswitch-default]
     switch (swindex1) {
            ^

I'll add the file as an attachment, too.

[Bug c/61864] Feature Request, -Wcovered-switch-default to identify "dead" default branch

[egall at gwmail dot gwu.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61864

--- Comment #5 from Eric Gallager <egall at gwmail dot gwu.edu> ---
Created attachment 35915
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35915&action=edit
test case demonstrating different switch-related warnings

[Bug c/61864] Feature Request, -Wcovered-switch-default to identify "dead" default branch

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61864

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.2                         |5.3

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 5.2 is being released, adjusting target milestone to 5.3.