Any comments on this patch? Should I pursue the C++ part?
On Tue, May 10, 2016 at 08:19:29PM +0200, Marek Polacek wrote:
> Over the years, we got several PRs that suggested to add a warning that would
> warn about unreachable statements between `switch (cond)' and the first case.
> In some cases our -Wuninitialized warning can detect such a case, but mostly
> not. This patch is an attempt to add a proper warning about this peculiar
> case. I chose to not warn about declarations between switch and the first
> case, because we use that in the codebase and I think this kind of use is
> fine. As it's usually the case, some obscure cases cropped up during testing,
> this time those were Duff's devices, so I had to go the extra mile to handle
> them specially.
>
> This is a C FE part only; I'd like to hear some feedback first before I plunge
> into the C++ FE.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2016-05-10 Marek Polacek <pola...@redhat.com>
>
> PR c/49859
> * c.opt (Wswitch-unreachable): New option.
>
> * c-parser.c: Include tree-iterator.h.
> (c_parser_switch_statement): Implement -Wswitch-unreachable warning.
>
> * doc/invoke.texi: Document -Wswitch-unreachable.
>
> * gcc.dg/Wjump-misses-init-1.c: Use -Wno-switch-unreachable.
> * gcc.dg/nested-func-1.c: Likewise.
> * gcc.dg/pr67784-4.c: Likewise.
> * gcc.dg/Wswitch-unreachable-1.c: New test.
> * gcc.dg/c99-vla-jump-5.c (f): Add dg-warning.
> * gcc.dg/switch-warn-1.c (foo1): Likewise.
> * c-c++-common/goacc/sb-2.c: Likewise.
> * gcc.dg/gomp/block-10.c: Likewise.
> * gcc.dg/gomp/block-9.c: Likewise.
> * gcc.dg/gomp/target-1.c: Likewise.
> * gcc.dg/gomp/target-2.c: Likewise.
> * gcc.dg/gomp/taskgroup-1.c: Likewise.
> * gcc.dg/gomp/teams-1.c: Likewise.
>
> diff --git gcc/c-family/c.opt gcc/c-family/c.opt
> index bdc6ee0..8b6fdbb 100644
> --- gcc/c-family/c.opt
> +++ gcc/c-family/c.opt
> @@ -634,6 +634,11 @@ Wswitch-bool
> C ObjC C++ ObjC++ Var(warn_switch_bool) Warning Init(1)
> Warn about switches with boolean controlling expression.
>
> +Wswitch-unreachable
> +C ObjC C++ ObjC++ Var(warn_switch_unreachable) Warning Init(1)
> +Warn about statements between switch's controlling expression and the first
> +case.
> +
> Wtemplates
> C++ ObjC++ Var(warn_templates) Warning
> Warn on primary template declaration.
> diff --git gcc/c/c-parser.c gcc/c/c-parser.c
> index 6523c08..bdf8e8e 100644
> --- gcc/c/c-parser.c
> +++ gcc/c/c-parser.c
> @@ -58,6 +58,7 @@ along with GCC; see the file COPYING3. If not see
> #include "c-family/c-indentation.h"
> #include "gimple-expr.h"
> #include "context.h"
> +#include "tree-iterator.h"
>
> /* We need to walk over decls with incomplete struct/union/enum types
> after parsing the whole translation unit.
> @@ -5605,7 +5606,30 @@ c_parser_switch_statement (c_parser *parser, bool
> *if_p)
> c_start_case (switch_loc, switch_cond_loc, expr, explicit_cast_p);
> save_break = c_break_label;
> c_break_label = NULL_TREE;
> + location_t first_loc = (c_parser_next_token_is (parser, CPP_OPEN_BRACE)
> + ? c_parser_peek_2nd_token (parser)->location
> + : c_parser_peek_token (parser)->location);
> body = c_parser_c99_block_statement (parser, if_p);
> + tree first = expr_first (TREE_CODE (body) == BIND_EXPR
> + ? BIND_EXPR_BODY (body) : body);
> + /* Statements between `switch' and the first case are never executed. */
> + if (warn_switch_unreachable
> + && first != NULL_TREE
> + && TREE_CODE (first) != CASE_LABEL_EXPR
> + && TREE_CODE (first) != LABEL_EXPR)
> + {
> + if (TREE_CODE (first) == DECL_EXPR
> + && DECL_INITIAL (DECL_EXPR_DECL (first)) == NULL_TREE)
> + /* Don't warn for a declaration, but warn for an initialization. */;
> + else if (TREE_CODE (first) == GOTO_EXPR
> + && TREE_CODE (GOTO_DESTINATION (first)) == LABEL_DECL
> + && DECL_ARTIFICIAL (GOTO_DESTINATION (first)))
> + /* Don't warn for compiler-generated gotos. These occur in Duff's
> + devices, for example. */;
> + else
> + warning_at (first_loc, OPT_Wswitch_unreachable,
> + "statement will never be executed");
> + }
> c_finish_case (body, ce.original_type);
> if (c_break_label)
> {
> diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
> index a54a0af..8f9c186 100644
> --- gcc/doc/invoke.texi
> +++ gcc/doc/invoke.texi
> @@ -297,7 +297,8 @@ Objective-C and Objective-C++ Dialects}.
> -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol
> -Wsuggest-final-types @gol -Wsuggest-final-methods -Wsuggest-override @gol
> -Wmissing-format-attribute -Wsubobject-linkage @gol
> --Wswitch -Wswitch-default -Wswitch-enum -Wswitch-bool -Wsync-nand @gol
> +-Wswitch -Wswitch-default -Wswitch-enum -Wswitch-bool @gol
> +-Wswitch-unreachable -Wsync-nand @gol
> -Wsystem-headers -Wtautological-compare -Wtrampolines -Wtrigraphs @gol
> -Wtype-limits -Wundef @gol
> -Wuninitialized -Wunknown-pragmas -Wunsafe-loop-optimizations @gol
> @@ -4135,6 +4136,39 @@ switch ((int) (a == 4))
> @end smallexample
> This warning is enabled by default for C and C++ programs.
>
> +@item -Wswitch-unreachable
> +@opindex Wswitch-unreachable
> +@opindex Wno-switch-unreachable
> +Warn whenever a @code{switch} statement contains statements between the
> +controlling expression and the first case label, which will never be
> +executed. For example:
> +@smallexample
> +@group
> +switch (cond)
> + @{
> + i = 15;
> + @dots{}
> + case 5:
> + @dots{}
> + @}
> +@end group
> +@end smallexample
> +@option{-Wswitch-unreachable} will not warn if the statement between the
> +controlling expression and the first case label is just a declaration:
> +@smallexample
> +@group
> +switch (cond)
> + @{
> + int i;
> + @dots{}
> + case 5:
> + i = 5;
> + @dots{}
> + @}
> +@end group
> +@end smallexample
> +This warning is enabled by default for C and C++ programs.
> +
> @item -Wsync-nand @r{(C and C++ only)}
> @opindex Wsync-nand
> @opindex Wno-sync-nand
> diff --git gcc/testsuite/c-c++-common/goacc/sb-2.c
> gcc/testsuite/c-c++-common/goacc/sb-2.c
> index a6760ec..e986af3 100644
> --- gcc/testsuite/c-c++-common/goacc/sb-2.c
> +++ gcc/testsuite/c-c++-common/goacc/sb-2.c
> @@ -4,19 +4,19 @@ void foo(int i)
> {
> switch (i) // { dg-error "invalid entry to OpenACC structured block" }
> {
> - #pragma acc parallel
> + #pragma acc parallel // { dg-warning "statement will never be executed" }
> { case 0:; }
> }
>
> switch (i) // { dg-error "invalid entry to OpenACC structured block" }
> {
> - #pragma acc kernels
> + #pragma acc kernels // { dg-warning "statement will never be executed" }
> { case 0:; }
> }
>
> switch (i) // { dg-error "invalid entry to OpenACC structured block" }
> {
> - #pragma acc data
> + #pragma acc data // { dg-warning "statement will never be executed" }
> { case 0:; }
> }
> }
> diff --git gcc/testsuite/gcc.dg/Wjump-misses-init-1.c
> gcc/testsuite/gcc.dg/Wjump-misses-init-1.c
> index 86117f1..71809be 100644
> --- gcc/testsuite/gcc.dg/Wjump-misses-init-1.c
> +++ gcc/testsuite/gcc.dg/Wjump-misses-init-1.c
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-Wjump-misses-init" } */
> +/* { dg-options "-Wjump-misses-init -Wno-switch-unreachable" } */
> int
> f1 (int a)
> {
> diff --git gcc/testsuite/gcc.dg/Wswitch-unreachable-1.c
> gcc/testsuite/gcc.dg/Wswitch-unreachable-1.c
> index e69de29..ec620d3 100644
> --- gcc/testsuite/gcc.dg/Wswitch-unreachable-1.c
> +++ gcc/testsuite/gcc.dg/Wswitch-unreachable-1.c
> @@ -0,0 +1,135 @@
> +/* PR c/49859 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wswitch-unreachable" } */
> +
> +extern void foo (int);
> +extern int j;
> +
> +void
> +fn0 (int i)
> +{
> + switch (i)
> + {
> + int k;
> + case 1:
> + k = 11;
> + foo (k);
> + }
> +
> + switch (i)
> + j = 10; /* { dg-warning "statement will never be executed" } */
> +
> + switch (i)
> + ;
> +
> + switch (i)
> + {
> + int t = 10; /* { dg-warning "statement will never be executed" } */
> + default:
> + foo (t);
> + }
> +
> + switch (i)
> + {
> + j = 12; /* { dg-warning "statement will never be executed" } */
> + default:
> + foo (j);
> + }
> +
> + int o;
> + switch (i)
> + {
> + o = 333; /* { dg-warning "statement will never be executed" } */
> + case 4: break;
> + default:
> + foo (o);
> + }
> +
> + switch (i)
> + switch (j) /* { dg-warning "statement will never be executed" } */
> + {
> + o = 42; /* { dg-warning "statement will never be executed" } */
> + case 8:;
> + }
> +
> + switch (i)
> + {
> + int l = 3; /* { dg-warning "statement will never be executed" } */
> + o = 5;
> + j = 7;
> + ++l;
> + }
> +
> + switch (i)
> + if (j != 3) /* { dg-warning "statement will never be executed" } */
> + foo (j);
> +
> + switch (i)
> + while (1)
> + foo (0);
> +
> + switch (i)
> + while (i > 5) { }
> +
> + switch (i)
> + goto X; /* { dg-warning "statement will never be executed" } */
> +X:
> +
> + switch (i)
> + do
> + foo (1);
> + while (1);
> +
> + switch (i)
> + for (;;)
> + foo (-1);
> +
> + switch (i)
> + default:
> + j = 6;
> +
> + switch (i)
> + default:
> + j = sizeof (struct { int i; });
> +
> + switch (i)
> + {
> + typedef int T;
> + case 3:
> + {
> + T x = 5;
> + foo (x);
> + }
> + }
> +
> + switch (i)
> + {
> + static int g;
> + default:
> + foo (g);
> + }
> +
> + switch (i)
> + {
> +L:
> + j = 16;
> + default:
> + if (j < 5)
> + goto L;
> + break;
> + }
> +
> + switch (i)
> + {
> + int A[i]; /* { dg-warning "statement will never be executed" } */
> + default: /* { dg-error "switch jumps into scope" } */
> + break;
> + }
> +
> + switch (i)
> + {
> + int A[3];
> + default:
> + break;
> + }
> +}
> diff --git gcc/testsuite/gcc.dg/c99-vla-jump-5.c
> gcc/testsuite/gcc.dg/c99-vla-jump-5.c
> index fc5e04d..b5b85fd 100644
> --- gcc/testsuite/gcc.dg/c99-vla-jump-5.c
> +++ gcc/testsuite/gcc.dg/c99-vla-jump-5.c
> @@ -15,7 +15,7 @@ void
> f (int a, int b)
> {
> switch (a) {
> - int v[b];
> + int v[b]; /* { dg-warning "statement will never be executed" } */
> case 2: /* { dg-error "switch jumps into scope of identifier with variably
> modified type" } */
> default: /* { dg-error "switch jumps into scope of identifier with
> variably modified type" } */
> switch (a)
> diff --git gcc/testsuite/gcc.dg/gomp/block-10.c
> gcc/testsuite/gcc.dg/gomp/block-10.c
> index 69ae3c0..29c2d91 100644
> --- gcc/testsuite/gcc.dg/gomp/block-10.c
> +++ gcc/testsuite/gcc.dg/gomp/block-10.c
> @@ -5,28 +5,28 @@ void foo(int i)
> int j;
> switch (i) // { dg-error "invalid entry to OpenMP structured block" }
> {
> - #pragma omp parallel
> + #pragma omp parallel // { dg-warning "statement will never be executed" }
> { case 0:; }
> }
> switch (i) // { dg-error "invalid entry to OpenMP structured block" }
> {
> - #pragma omp for
> + #pragma omp for // { dg-warning "statement will never be executed" }
> for (j = 0; j < 10; ++ j)
> { case 1:; }
> }
> switch (i) // { dg-error "invalid entry to OpenMP structured block" }
> {
> - #pragma omp critical
> + #pragma omp critical // { dg-warning "statement will never be executed" }
> { case 2:; }
> }
> switch (i) // { dg-error "invalid entry to OpenMP structured block" }
> {
> - #pragma omp master
> + #pragma omp master // { dg-warning "statement will never be executed" }
> { case 3:; }
> }
> switch (i) // { dg-error "invalid entry to OpenMP structured block" }
> {
> - #pragma omp sections
> + #pragma omp sections // { dg-warning "statement will never be executed" }
> { case 4:;
> #pragma omp section
> { case 5:; }
> @@ -34,7 +34,7 @@ void foo(int i)
> }
> switch (i) // { dg-error "invalid entry to OpenMP structured block" }
> {
> - #pragma omp ordered
> + #pragma omp ordered // { dg-warning "statement will never be executed" }
> { default:; }
> }
> }
> diff --git gcc/testsuite/gcc.dg/gomp/block-9.c
> gcc/testsuite/gcc.dg/gomp/block-9.c
> index 2fae3de..202599f 100644
> --- gcc/testsuite/gcc.dg/gomp/block-9.c
> +++ gcc/testsuite/gcc.dg/gomp/block-9.c
> @@ -5,7 +5,7 @@ void foo(int i)
> int j;
> switch (i) // { dg-error "invalid entry to OpenMP structured block" }
> {
> - #pragma omp parallel
> + #pragma omp parallel // { dg-warning "statement will never be executed" }
> { case 0:; }
> #pragma omp for
> for (j = 0; j < 10; ++ j)
> diff --git gcc/testsuite/gcc.dg/gomp/target-1.c
> gcc/testsuite/gcc.dg/gomp/target-1.c
> index aaa6a14..6bc5eb9 100644
> --- gcc/testsuite/gcc.dg/gomp/target-1.c
> +++ gcc/testsuite/gcc.dg/gomp/target-1.c
> @@ -23,7 +23,7 @@ foo (int x)
>
> switch (x) // { dg-error "invalid entry to OpenMP structured block" }
> {
> - #pragma omp target
> + #pragma omp target // { dg-warning "statement will never be executed" }
> { case 0:; }
> }
> }
> diff --git gcc/testsuite/gcc.dg/gomp/target-2.c
> gcc/testsuite/gcc.dg/gomp/target-2.c
> index 3a7afc4..c5c38d8 100644
> --- gcc/testsuite/gcc.dg/gomp/target-2.c
> +++ gcc/testsuite/gcc.dg/gomp/target-2.c
> @@ -23,7 +23,7 @@ foo (int x, int y)
>
> switch (x) // { dg-error "invalid entry to OpenMP structured block" }
> {
> - #pragma omp target data map(tofrom: y)
> + #pragma omp target data map(tofrom: y) // { dg-warning "statement will
> never be executed" }
> { case 0:; }
> }
> }
> diff --git gcc/testsuite/gcc.dg/gomp/taskgroup-1.c
> gcc/testsuite/gcc.dg/gomp/taskgroup-1.c
> index 1997e0c..f39b7ef 100644
> --- gcc/testsuite/gcc.dg/gomp/taskgroup-1.c
> +++ gcc/testsuite/gcc.dg/gomp/taskgroup-1.c
> @@ -23,7 +23,7 @@ foo (int x)
>
> switch (x) // { dg-error "invalid entry to OpenMP structured block" }
> {
> - #pragma omp taskgroup
> + #pragma omp taskgroup // { dg-warning "statement will never be executed" }
> { case 0:; }
> }
> }
> diff --git gcc/testsuite/gcc.dg/gomp/teams-1.c
> gcc/testsuite/gcc.dg/gomp/teams-1.c
> index ad5b100..db7f50b 100644
> --- gcc/testsuite/gcc.dg/gomp/teams-1.c
> +++ gcc/testsuite/gcc.dg/gomp/teams-1.c
> @@ -23,7 +23,7 @@ foo (int x)
>
> switch (x) // { dg-error "invalid entry to OpenMP structured block" }
> {
> - #pragma omp target teams
> + #pragma omp target teams // { dg-warning "statement will never be
> executed" }
> { case 0:; }
> }
> }
> @@ -54,7 +54,7 @@ bar (int x)
>
> switch (x) // { dg-error "invalid entry to OpenMP structured block" }
> {
> - #pragma omp target
> + #pragma omp target // { dg-warning "statement will never be executed" }
> #pragma omp teams
> { case 0:; }
> }
> diff --git gcc/testsuite/gcc.dg/nested-func-1.c
> gcc/testsuite/gcc.dg/nested-func-1.c
> index cb26e89..2052a6f 100644
> --- gcc/testsuite/gcc.dg/nested-func-1.c
> +++ gcc/testsuite/gcc.dg/nested-func-1.c
> @@ -1,7 +1,7 @@
> /* Test for proper errors for break and continue in nested functions. */
> /* Origin: Joseph Myers <j...@polyomino.org.uk> */
> /* { dg-do compile } */
> -/* { dg-options "" } */
> +/* { dg-options "-Wno-switch-unreachable" } */
>
> void
> foo (int a)
> diff --git gcc/testsuite/gcc.dg/pr67784-4.c gcc/testsuite/gcc.dg/pr67784-4.c
> index 81a43fd..5462080 100644
> --- gcc/testsuite/gcc.dg/pr67784-4.c
> +++ gcc/testsuite/gcc.dg/pr67784-4.c
> @@ -1,6 +1,6 @@
> /* PR c/67784 */
> /* { dg-do compile } */
> -/* { dg-options "" } */
> +/* { dg-options "-Wno-switch-unreachable" } */
>
> typedef int T;
>
> diff --git gcc/testsuite/gcc.dg/switch-warn-1.c
> gcc/testsuite/gcc.dg/switch-warn-1.c
> index 04ca4e3..58fbd7d 100644
> --- gcc/testsuite/gcc.dg/switch-warn-1.c
> +++ gcc/testsuite/gcc.dg/switch-warn-1.c
> @@ -11,7 +11,7 @@ foo1 (unsigned char i)
> {
> switch (i)
> {
> - case -1: /* { dg-warning "case label value is less than minimum value
> for type" } */
> + case -1: /* { dg-warning "case label value is less than minimum value
> for type|statement will never be executed" } */
> return 1;
> case 256: /* { dg-warning "case label value exceeds maximum value for
> type" } */
> return 2;
>
> Marek
Marek
