* C PATCH to add -Wswitch-unreachable (PR c/49859)
@ 2016-05-10 18:19 Marek Polacek
2016-05-19 15:36 ` Marek Polacek
2016-05-19 15:54 ` Jason Merrill
0 siblings, 2 replies; 15+ messages in thread
From: Marek Polacek @ 2016-05-10 18:19 UTC (permalink / raw)
To: GCC Patches; +Cc: Joseph Myers
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 <polacek@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 <jsm@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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: C PATCH to add -Wswitch-unreachable (PR c/49859)
2016-05-10 18:19 C PATCH to add -Wswitch-unreachable (PR c/49859) Marek Polacek
@ 2016-05-19 15:36 ` Marek Polacek
2016-05-19 15:54 ` Jason Merrill
1 sibling, 0 replies; 15+ messages in thread
From: Marek Polacek @ 2016-05-19 15:36 UTC (permalink / raw)
To: GCC Patches; +Cc: Joseph Myers
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 <polacek@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 <jsm@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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: C PATCH to add -Wswitch-unreachable (PR c/49859)
2016-05-10 18:19 C PATCH to add -Wswitch-unreachable (PR c/49859) Marek Polacek
2016-05-19 15:36 ` Marek Polacek
@ 2016-05-19 15:54 ` Jason Merrill
2016-05-20 16:36 ` Marek Polacek
1 sibling, 1 reply; 15+ messages in thread
From: Jason Merrill @ 2016-05-19 15:54 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Joseph Myers
Why implement this in the front end rather than at the gimple level?
Jason
On Tue, May 10, 2016 at 2:19 PM, Marek Polacek <polacek@redhat.com> 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 <polacek@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 <jsm@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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: C PATCH to add -Wswitch-unreachable (PR c/49859)
2016-05-19 15:54 ` Jason Merrill
@ 2016-05-20 16:36 ` Marek Polacek
2016-05-20 16:53 ` Jason Merrill
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Marek Polacek @ 2016-05-20 16:36 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches, Joseph Myers
On Thu, May 19, 2016 at 11:53:52AM -0400, Jason Merrill wrote:
> Why implement this in the front end rather than at the gimple level?
I was afraid that I wouldn't have as good a location info as in the FE and
I wasn't sure if I'd be able to handle declarations well.
Now that I've rewritten this to GIMPLE, I no longer fear so. Locations are
ok for various gimple_assigns and I don't have to care about DECL_EXPRs.
Moreover, this works fine even for the C++ FE now. (But I had to disable the
warning for Fortran.)
I also discovered a bug in my previous version - it wouldn't warn for
switch (i)
{
int i;
int j = 10;
case 4: ...
}
because it only looked at the first statement after switch (i) and punted
for DECL_EXPRs. This now works as it should.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2016-05-20 Marek Polacek <polacek@redhat.com>
PR c/49859
* c.opt (Wswitch-unreachable): New option.
* doc/invoke.texi: Document -Wswitch-unreachable.
* gimplify.c (gimplify_switch_expr): Implement the -Wswitch-unreachable
warning.
* c-c++-common/Wswitch-unreachable-1.c: New test.
* gcc.dg/Wswitch-unreachable-1.c: New test.
* c-c++-common/goacc/sb-2.c (void foo): Add dg-warning.
* g++.dg/cpp0x/lambda/lambda-switch.C (main): Likewise.
* g++.dg/gomp/block-10.C: Likewise.
* gcc.dg/gomp/block-10.c: Likewise.
* g++.dg/gomp/block-9.C: Likewise.
* gcc.dg/gomp/block-9.c: Likewise.
* g++.dg/gomp/target-1.C: Likewise.
* g++.dg/gomp/target-2.C: Likewise.
* gcc.dg/gomp/target-1.c: Likewise.
* gcc.dg/gomp/target-2.c: Likewise.
* g++.dg/gomp/taskgroup-1.C: Likewise.
* gcc.dg/gomp/taskgroup-1.c: Likewise.
* gcc.dg/gomp/teams-1.c: Likewise.
* g++.dg/gomp/teams-1.C: Likewise.
* g++.dg/overload/error3.C: Likewise.
* g++.dg/tm/jump1.C: Likewise.
* g++.dg/torture/pr40335.C: Likewise.
* gcc.dg/c99-vla-jump-5.c: Likewise.
* gcc.dg/switch-warn-1.c: Likewise.
* gcc.dg/Wjump-misses-init-1.c: Use -Wno-switch-unreachable.
* gcc.dg/nested-func-1.c: Likewise.
* gcc.dg/pr67784-4.c: Likewise.
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 918df16..ed98503 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/doc/invoke.texi gcc/doc/invoke.texi
index f3d087f..5909b9d 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
@@ -4144,6 +4145,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/gimplify.c gcc/gimplify.c
index c433a84..57eca85 100644
--- gcc/gimplify.c
+++ gcc/gimplify.c
@@ -1595,6 +1595,32 @@ gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p)
gimplify_ctxp->case_labels.create (8);
gimplify_stmt (&SWITCH_BODY (switch_expr), &switch_body_seq);
+
+ /* Possibly warn about unreachable statements between switch's
+ controlling expression and the first case. */
+ if (warn_switch_unreachable
+ /* This warning doesn't play well with Fortran when optimizations
+ are on. */
+ && !lang_GNU_Fortran ()
+ && switch_body_seq != NULL)
+ {
+ gimple_seq seq = switch_body_seq;
+ if (gimple_code (switch_body_seq) == GIMPLE_BIND)
+ seq = gimple_bind_body (as_a <gbind *> (switch_body_seq));
+ gimple *stmt = gimple_seq_first_stmt (seq);
+ enum gimple_code code = gimple_code (stmt);
+ if (code != GIMPLE_LABEL && code != GIMPLE_TRY)
+ {
+ if (code == GIMPLE_GOTO
+ && TREE_CODE (gimple_goto_dest (stmt)) == LABEL_DECL
+ && DECL_ARTIFICIAL (gimple_goto_dest (stmt)))
+ /* Don't warn for compiler-generated gotos. These occur
+ in Duff's devices, for example. */;
+ else
+ warning_at (gimple_location (stmt), OPT_Wswitch_unreachable,
+ "statement will never be executed");
+ }
+ }
labels = gimplify_ctxp->case_labels;
gimplify_ctxp->case_labels = saved_labels;
diff --git gcc/testsuite/c-c++-common/Wswitch-unreachable-1.c gcc/testsuite/c-c++-common/Wswitch-unreachable-1.c
index e69de29..ee6ecc1 100644
--- gcc/testsuite/c-c++-common/Wswitch-unreachable-1.c
+++ gcc/testsuite/c-c++-common/Wswitch-unreachable-1.c
@@ -0,0 +1,116 @@
+/* PR c/49859 */
+/* { dg-do compile } */
+
+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)
+ {
+ 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)
+ {
+ int x;
+ int l = 3; /* { dg-warning "statement will never be executed" } */
+ ++l, ++x;
+ }
+
+ 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)
+ {
+ 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;
+ }
+}
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/g++.dg/cpp0x/lambda/lambda-switch.C gcc/testsuite/g++.dg/cpp0x/lambda/lambda-switch.C
index 1cac211..d71d3ad 100644
--- gcc/testsuite/g++.dg/cpp0x/lambda/lambda-switch.C
+++ gcc/testsuite/g++.dg/cpp0x/lambda/lambda-switch.C
@@ -20,7 +20,7 @@ main ()
{
case 3: // { dg-error "case" }
break; // { dg-error "break" }
- };
+ }; // { dg-warning "statement will never be executed" }
}
}
}
diff --git gcc/testsuite/g++.dg/gomp/block-10.C gcc/testsuite/g++.dg/gomp/block-10.C
index b273c1f..4aa41cd 100644
--- gcc/testsuite/g++.dg/gomp/block-10.C
+++ gcc/testsuite/g++.dg/gomp/block-10.C
@@ -5,28 +5,28 @@ void foo(int i)
int j;
switch (i)
{
- #pragma omp parallel
+ #pragma omp parallel // { dg-warning "statement will never be executed" }
{ case 0:; } // { dg-error "jump|enters" }
}
switch (i)
{
- #pragma omp for
+ #pragma omp for // { dg-warning "statement will never be executed" }
for (j = 0; j < 10; ++ j)
{ case 1:; } // { dg-error "jump|enters" }
}
switch (i)
{
- #pragma omp critical
+ #pragma omp critical // { dg-warning "statement will never be executed" }
{ case 2:; } // { dg-error "jump|enters" }
}
switch (i)
{
- #pragma omp master
+ #pragma omp master // { dg-warning "statement will never be executed" }
{ case 3:; } // { dg-error "jump|enters" }
}
switch (i)
{
- #pragma omp sections
+ #pragma omp sections // { dg-warning "statement will never be executed" }
{ case 4:; // { dg-error "jump|enters" }
#pragma omp section
{ case 5:; } // { dg-error "jump|enters" }
@@ -34,7 +34,7 @@ void foo(int i)
}
switch (i)
{
- #pragma omp ordered
+ #pragma omp ordered // { dg-warning "statement will never be executed" }
{ default:; } // { dg-error "jump|enters" }
}
}
diff --git gcc/testsuite/g++.dg/gomp/block-9.C gcc/testsuite/g++.dg/gomp/block-9.C
index 8012e5a..1de40a9 100644
--- gcc/testsuite/g++.dg/gomp/block-9.C
+++ gcc/testsuite/g++.dg/gomp/block-9.C
@@ -5,7 +5,7 @@ void foo(int i)
int j;
switch (i)
{
- #pragma omp parallel
+ #pragma omp parallel // { dg-warning "statement will never be executed" }
{ case 0:; } // { dg-error "jump|enters" }
#pragma omp for
for (j = 0; j < 10; ++ j)
diff --git gcc/testsuite/g++.dg/gomp/target-1.C gcc/testsuite/g++.dg/gomp/target-1.C
index bcdac61..9750873 100644
--- gcc/testsuite/g++.dg/gomp/target-1.C
+++ gcc/testsuite/g++.dg/gomp/target-1.C
@@ -24,7 +24,7 @@ foo (int x)
switch (x)
{
- #pragma omp target
+ #pragma omp target // { dg-warning "statement will never be executed" }
{ case 0:; } // { dg-error "jump" }
// { dg-message "enters" "" { target *-*-* } 28 }
}
diff --git gcc/testsuite/g++.dg/gomp/target-2.C gcc/testsuite/g++.dg/gomp/target-2.C
index 273f8d5..333b4d0 100644
--- gcc/testsuite/g++.dg/gomp/target-2.C
+++ gcc/testsuite/g++.dg/gomp/target-2.C
@@ -24,7 +24,7 @@ foo (int x, int y)
switch (x)
{
- #pragma omp target data map(tofrom: y)
+ #pragma omp target data map(tofrom: y) // { dg-warning "statement will never be executed" }
{ case 0:; } // { dg-error "jump" }
// { dg-message "enters" "" { target *-*-* } 28 }
}
diff --git gcc/testsuite/g++.dg/gomp/taskgroup-1.C gcc/testsuite/g++.dg/gomp/taskgroup-1.C
index e15d59d..5542a4e 100644
--- gcc/testsuite/g++.dg/gomp/taskgroup-1.C
+++ gcc/testsuite/g++.dg/gomp/taskgroup-1.C
@@ -24,7 +24,7 @@ foo (int x)
switch (x)
{
- #pragma omp taskgroup
+ #pragma omp taskgroup // { dg-warning "statement will never be executed" }
{ case 0:; } // { dg-error "jump" }
// { dg-message "enters" "" { target *-*-* } 28 }
}
diff --git gcc/testsuite/g++.dg/gomp/teams-1.C gcc/testsuite/g++.dg/gomp/teams-1.C
index 2b00bb6..d0460c3 100644
--- gcc/testsuite/g++.dg/gomp/teams-1.C
+++ gcc/testsuite/g++.dg/gomp/teams-1.C
@@ -26,6 +26,7 @@ foo (int x)
{
#pragma omp target teams
{ case 0:; } // { dg-error "jump" }
+ // { dg-warning "statement will never be executed" "" { target *-*-* } 28 }
// { dg-message "enters" "" { target *-*-* } 28 }
}
}
@@ -43,7 +44,7 @@ bar (int x)
#pragma omp teams
{
bad2: ; // { dg-error "jump to label" }
- // { dg-message "enters OpenMP" "" { target *-*-* } 45 }
+ // { dg-message "enters OpenMP" "" { target *-*-* } 46 }
}
#pragma omp target
@@ -57,14 +58,14 @@ bar (int x)
switch (x)
{
- #pragma omp target
+ #pragma omp target // { dg-warning "statement will never be executed" }
#pragma omp teams
{ case 0:; } // { dg-error "jump" }
- // { dg-message "enters" "" { target *-*-* } 62 }
+ // { dg-message "enters" "" { target *-*-* } 63 }
}
}
// { dg-error "invalid branch to/from OpenMP structured block" "" { target *-*-* } 8 }
// { dg-error "invalid entry to OpenMP structured block" "" { target *-*-* } 10 }
-// { dg-error "invalid branch to/from OpenMP structured block" "" { target *-*-* } 39 }
-// { dg-error "invalid entry to OpenMP structured block" "" { target *-*-* } 41 }
+// { dg-error "invalid branch to/from OpenMP structured block" "" { target *-*-* } 40 }
+// { dg-error "invalid entry to OpenMP structured block" "" { target *-*-* } 42 }
diff --git gcc/testsuite/g++.dg/overload/error3.C gcc/testsuite/g++.dg/overload/error3.C
index e0003dd..8391875 100644
--- gcc/testsuite/g++.dg/overload/error3.C
+++ gcc/testsuite/g++.dg/overload/error3.C
@@ -35,6 +35,7 @@ class MainWindow {
void MainWindow::update_status(Result result) {
switch (result) {
status_frame.modify_bg(Gtk::STATE_NORMAL,Gdk::Color::Color("green")); // { dg-error "" }
+ // { dg-warning "statement will never be executed" "" { target *-*-* } 37 }
status_frame.modify_bg(Gtk::STATE_NORMAL,Gdk::Color::Color("red")); // { dg-error "" }
status_label.set_text("Out of memory");
}
diff --git gcc/testsuite/g++.dg/tm/jump1.C gcc/testsuite/g++.dg/tm/jump1.C
index 003eed0..e28282d 100644
--- gcc/testsuite/g++.dg/tm/jump1.C
+++ gcc/testsuite/g++.dg/tm/jump1.C
@@ -14,7 +14,7 @@ void f()
switch (i)
{
- synchronized {
+ synchronized { // { dg-warning "statement will never be executed" }
++i;
case 42: // { dg-error "" }
++i;
diff --git gcc/testsuite/g++.dg/torture/pr40335.C gcc/testsuite/g++.dg/torture/pr40335.C
index 14ea95d..295a356 100644
--- gcc/testsuite/g++.dg/torture/pr40335.C
+++ gcc/testsuite/g++.dg/torture/pr40335.C
@@ -8,7 +8,7 @@ main (void)
switch ((signed char) i)
{
case 255: /* { dg-bogus "exceeds maximum value" "" { xfail *-*-* } } */
- abort ();
+ abort (); /* { dg-warning "statement will never be executed" } */
default:
break;
}
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..2e5c99b 100644
--- gcc/testsuite/gcc.dg/Wswitch-unreachable-1.c
+++ gcc/testsuite/gcc.dg/Wswitch-unreachable-1.c
@@ -0,0 +1,35 @@
+/* PR c/49859 */
+/* { dg-do compile } */
+/* { dg-options "-Wswitch-unreachable" } */
+
+extern void foo (int);
+extern int j;
+
+void
+fn0 (int i)
+{
+ switch (i)
+ {
+ int t = 10; /* { dg-warning "statement will never be executed" } */
+ default:
+ foo (t);
+ }
+
+ switch (i)
+ { /* { dg-warning "statement will never be executed" } */
+ int A[i];
+ default: /* { dg-error "switch jumps into scope" } */
+ break;
+ }
+
+ switch (i)
+ default:
+ j = sizeof (struct { int i; });
+
+ 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..5b5fc74 100644
--- gcc/testsuite/gcc.dg/c99-vla-jump-5.c
+++ gcc/testsuite/gcc.dg/c99-vla-jump-5.c
@@ -14,7 +14,7 @@
void
f (int a, int b)
{
- switch (a) {
+ switch (a) { /* { dg-warning "statement will never be executed" } */
int v[b];
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" } */
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..a537047 100644
--- gcc/testsuite/gcc.dg/gomp/teams-1.c
+++ gcc/testsuite/gcc.dg/gomp/teams-1.c
@@ -24,7 +24,7 @@ foo (int x)
switch (x) // { dg-error "invalid entry to OpenMP structured block" }
{
#pragma omp target teams
- { case 0:; }
+ { case 0:; } // { dg-warning "statement will never be executed" }
}
}
@@ -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 <jsm@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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: C PATCH to add -Wswitch-unreachable (PR c/49859)
2016-05-20 16:36 ` Marek Polacek
@ 2016-05-20 16:53 ` Jason Merrill
2016-05-21 2:17 ` Sandra Loosemore
` (2 subsequent siblings)
3 siblings, 0 replies; 15+ messages in thread
From: Jason Merrill @ 2016-05-20 16:53 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Joseph Myers
On 05/20/2016 12:36 PM, Marek Polacek wrote:
> PR c/49859
> * c.opt (Wswitch-unreachable): New option.
This should go in common.opt, since the flag variable is used in
language-independent code. OK with that change.
Jason
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: C PATCH to add -Wswitch-unreachable (PR c/49859)
2016-05-20 16:36 ` Marek Polacek
2016-05-20 16:53 ` Jason Merrill
@ 2016-05-21 2:17 ` Sandra Loosemore
2016-05-23 9:55 ` Marek Polacek
2016-05-23 9:43 ` Florian Weimer
2016-05-23 23:53 ` Martin Sebor
3 siblings, 1 reply; 15+ messages in thread
From: Sandra Loosemore @ 2016-05-21 2:17 UTC (permalink / raw)
To: Marek Polacek, Jason Merrill; +Cc: GCC Patches, Joseph Myers
On 05/20/2016 10:36 AM, Marek Polacek wrote:
> diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
> index f3d087f..5909b9d 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
I think this list is supposed to be alphabetized except with respect to
-Wno-foo being sorted as if it were -Wfoo. I realize there are other
inconsistencies, but can you at least keep the -Wswitch* entries in
proper order?
> @@ -4144,6 +4145,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
s/will/does/
> +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
The doc part of the patch is OK with those things fixed.
-Sandra
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: C PATCH to add -Wswitch-unreachable (PR c/49859)
2016-05-20 16:36 ` Marek Polacek
2016-05-20 16:53 ` Jason Merrill
2016-05-21 2:17 ` Sandra Loosemore
@ 2016-05-23 9:43 ` Florian Weimer
2016-05-23 10:17 ` Marek Polacek
2016-05-23 23:53 ` Martin Sebor
3 siblings, 1 reply; 15+ messages in thread
From: Florian Weimer @ 2016-05-23 9:43 UTC (permalink / raw)
To: Marek Polacek, Jason Merrill; +Cc: GCC Patches, Joseph Myers
On 05/20/2016 06:36 PM, Marek Polacek wrote:
> --- gcc/gimplify.c
> +++ gcc/gimplify.c
> @@ -1595,6 +1595,32 @@ gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p)
> gimplify_ctxp->case_labels.create (8);
>
> gimplify_stmt (&SWITCH_BODY (switch_expr), &switch_body_seq);
> +
> + /* Possibly warn about unreachable statements between switch's
> + controlling expression and the first case. */
> + if (warn_switch_unreachable
> + /* This warning doesn't play well with Fortran when optimizations
> + are on. */
> + && !lang_GNU_Fortran ()
> + && switch_body_seq != NULL)
Does this still make the warning dependent on the optimization level and
the target?
Can we document what a programmer needs to do to suppress the warning?
My worry is that the warning will be too unpredictable to be useful.
Thanks,
Florian
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: C PATCH to add -Wswitch-unreachable (PR c/49859)
2016-05-21 2:17 ` Sandra Loosemore
@ 2016-05-23 9:55 ` Marek Polacek
0 siblings, 0 replies; 15+ messages in thread
From: Marek Polacek @ 2016-05-23 9:55 UTC (permalink / raw)
To: Sandra Loosemore; +Cc: Jason Merrill, GCC Patches, Joseph Myers
On Fri, May 20, 2016 at 08:17:50PM -0600, Sandra Loosemore wrote:
> On 05/20/2016 10:36 AM, Marek Polacek wrote:
> > diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
> > index f3d087f..5909b9d 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
>
> I think this list is supposed to be alphabetized except with respect to
> -Wno-foo being sorted as if it were -Wfoo. I realize there are other
> inconsistencies, but can you at least keep the -Wswitch* entries in proper
> order?
Sure, fixed.
> > @@ -4144,6 +4145,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
>
> s/will/does/
OK.
> > +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
>
> The doc part of the patch is OK with those things fixed.
Thanks, I made the changes.
Marek
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: C PATCH to add -Wswitch-unreachable (PR c/49859)
2016-05-23 9:43 ` Florian Weimer
@ 2016-05-23 10:17 ` Marek Polacek
2016-05-30 12:33 ` Florian Weimer
0 siblings, 1 reply; 15+ messages in thread
From: Marek Polacek @ 2016-05-23 10:17 UTC (permalink / raw)
To: Florian Weimer; +Cc: Jason Merrill, GCC Patches, Joseph Myers
On Mon, May 23, 2016 at 11:43:24AM +0200, Florian Weimer wrote:
> On 05/20/2016 06:36 PM, Marek Polacek wrote:
> > --- gcc/gimplify.c
> > +++ gcc/gimplify.c
> > @@ -1595,6 +1595,32 @@ gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p)
> > gimplify_ctxp->case_labels.create (8);
> >
> > gimplify_stmt (&SWITCH_BODY (switch_expr), &switch_body_seq);
> > +
> > + /* Possibly warn about unreachable statements between switch's
> > + controlling expression and the first case. */
> > + if (warn_switch_unreachable
> > + /* This warning doesn't play well with Fortran when optimizations
> > + are on. */
> > + && !lang_GNU_Fortran ()
> > + && switch_body_seq != NULL)
>
> Does this still make the warning dependent on the optimization level and the
> target?
It's not dependent on the optimization level for C/C++ and I don't quite see
why would it be dependent on the target. It's just that the Fortran FE with
-O optimizes a switch by removing the first case. I don't know Fortran to
judge whether the warning could be useful there.
> Can we document what a programmer needs to do to suppress the warning? My
> worry is that the warning will be too unpredictable to be useful.
Why would you suppress the warning? Just delete the code that cannot be
executed anyway. And what is unpredictable? You don't want the compiler
to warn on
switch(x) {
found = 1;
case 1:
...
?
Marek
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: C PATCH to add -Wswitch-unreachable (PR c/49859)
2016-05-20 16:36 ` Marek Polacek
` (2 preceding siblings ...)
2016-05-23 9:43 ` Florian Weimer
@ 2016-05-23 23:53 ` Martin Sebor
2016-05-25 17:20 ` Marek Polacek
3 siblings, 1 reply; 15+ messages in thread
From: Martin Sebor @ 2016-05-23 23:53 UTC (permalink / raw)
To: Marek Polacek, Jason Merrill; +Cc: GCC Patches, Joseph Myers
Sorry I'm a little late with my comments but I noticed one minor
problem (I raised bug 71249 for it since the patch has already
been checked in), and have a question about the hunk below:
> @@ -1595,6 +1595,32 @@ gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p)
> gimplify_ctxp->case_labels.create (8);
>
> gimplify_stmt (&SWITCH_BODY (switch_expr), &switch_body_seq);
> +
> + /* Possibly warn about unreachable statements between switch's
> + controlling expression and the first case. */
> + if (warn_switch_unreachable
> + /* This warning doesn't play well with Fortran when optimizations
> + are on. */
> + && !lang_GNU_Fortran ()
> + && switch_body_seq != NULL)
> + {
> + gimple_seq seq = switch_body_seq;
> + if (gimple_code (switch_body_seq) == GIMPLE_BIND)
> + seq = gimple_bind_body (as_a <gbind *> (switch_body_seq));
> + gimple *stmt = gimple_seq_first_stmt (seq);
> + enum gimple_code code = gimple_code (stmt);
> + if (code != GIMPLE_LABEL && code != GIMPLE_TRY)
Why exempt GIMPLE_TRY? It suppresses the warning in cases like:
switch (i) {
try { } catch (...) { }
case 1: ;
}
(If excluding GIMPLE_TRY is unavoidable, it might be worthwhile
to add a comment to the code, and perhaps also mention it in
the documentation to preempt bug reports by nitpickers like me ;)
Finally, while even this simple warning can be useful, it would
be even more helpful if it could also point out other unreachable
statements within the body of the switch statements after
a break/goto/return and before a subsequent label. This could
be especially valuable with optimization to make possible
diagnosing non-trivial problems like this:
switch (i) {
case 3:
if (i < 3)
return 1;
i = 8;
}
(I realize this might be outside the scope of the feature request
and starting to creep into the -Wunreachable-code territory.)
Martin
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: C PATCH to add -Wswitch-unreachable (PR c/49859)
2016-05-23 23:53 ` Martin Sebor
@ 2016-05-25 17:20 ` Marek Polacek
2016-05-26 8:27 ` Martin Sebor
0 siblings, 1 reply; 15+ messages in thread
From: Marek Polacek @ 2016-05-25 17:20 UTC (permalink / raw)
To: Martin Sebor; +Cc: Jason Merrill, GCC Patches, Joseph Myers
On Mon, May 23, 2016 at 05:53:38PM -0600, Martin Sebor wrote:
> Sorry I'm a little late with my comments but I noticed one minor
> problem (I raised bug 71249 for it since the patch has already
> been checked in), and have a question about the hunk below:
Firstly, thanks for looking into this.
> > @@ -1595,6 +1595,32 @@ gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p)
> > gimplify_ctxp->case_labels.create (8);
> >
> > gimplify_stmt (&SWITCH_BODY (switch_expr), &switch_body_seq);
> > +
> > + /* Possibly warn about unreachable statements between switch's
> > + controlling expression and the first case. */
> > + if (warn_switch_unreachable
> > + /* This warning doesn't play well with Fortran when optimizations
> > + are on. */
> > + && !lang_GNU_Fortran ()
> > + && switch_body_seq != NULL)
> > + {
> > + gimple_seq seq = switch_body_seq;
> > + if (gimple_code (switch_body_seq) == GIMPLE_BIND)
> > + seq = gimple_bind_body (as_a <gbind *> (switch_body_seq));
> > + gimple *stmt = gimple_seq_first_stmt (seq);
> > + enum gimple_code code = gimple_code (stmt);
> > + if (code != GIMPLE_LABEL && code != GIMPLE_TRY)
>
> Why exempt GIMPLE_TRY? It suppresses the warning in cases like:
>
> switch (i) {
> try { } catch (...) { }
> case 1: ;
> }
>
> (If excluding GIMPLE_TRY is unavoidable, it might be worthwhile
> to add a comment to the code, and perhaps also mention it in
> the documentation to preempt bug reports by nitpickers like me ;)
I think I added that so that we do not warn on
switch (i)
{
int A[3];
default:
break;
}
because at the gimple level that looks like
{
int A[3];
try
{
<D.1751>:
goto <D.1752>;
}
finally
{
A = {CLOBBER};
}
}
Another problem with try/finally is that it doesn't have a location
so we'd jsut print useless
cc1: warning: statement will never be executed [-Wswitch-unreachable]
Though it seems so improbable that I don't really care about this case.
> Finally, while even this simple warning can be useful, it would
> be even more helpful if it could also point out other unreachable
> statements within the body of the switch statements after
> a break/goto/return and before a subsequent label. This could
> be especially valuable with optimization to make possible
> diagnosing non-trivial problems like this:
>
> switch (i) {
> case 3:
> if (i < 3)
> return 1;
> i = 8;
> }
>
> (I realize this might be outside the scope of the feature request
> and starting to creep into the -Wunreachable-code territory.)
This really sounds like the old -Wunreachable stuff and I don't think
it's limited to switches as this warning is. Nowadays we have stuff
like gimple_stmt_may_fallthru so maybe that could be useful, but I'm
not about to plunge into this mess anytime soon ;).
Marek
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: C PATCH to add -Wswitch-unreachable (PR c/49859)
2016-05-25 17:20 ` Marek Polacek
@ 2016-05-26 8:27 ` Martin Sebor
2016-05-26 9:32 ` Marek Polacek
0 siblings, 1 reply; 15+ messages in thread
From: Martin Sebor @ 2016-05-26 8:27 UTC (permalink / raw)
To: Marek Polacek; +Cc: Jason Merrill, GCC Patches, Joseph Myers
>> Why exempt GIMPLE_TRY? It suppresses the warning in cases like:
>>
>> switch (i) {
>> try { } catch (...) { }
>> case 1: ;
>> }
>>
>> (If excluding GIMPLE_TRY is unavoidable, it might be worthwhile
>> to add a comment to the code, and perhaps also mention it in
>> the documentation to preempt bug reports by nitpickers like me ;)
>
> I think I added that so that we do not warn on
> switch (i)
> {
> int A[3];
> default:
> break;
> }
> because at the gimple level that looks like
> {
> int A[3];
>
> try
> {
> <D.1751>:
> goto <D.1752>;
> }
> finally
> {
> A = {CLOBBER};
> }
> }
I see. Thanks for clarifying that. No warning on a declaration
alone makes sense in the case above but it has the unfortunate
effect of suppressing the warning when the declaration is followed
by a statement, such as in:
void f (int*, int);
void g (int i)
{
switch (i) {
int a [3];
memset (a, 0, sizeof a);
default:
f (a, 3);
}
}
> Another problem with try/finally is that it doesn't have a location
> so we'd jsut print useless
> cc1: warning: statement will never be executed [-Wswitch-unreachable]
That's too bad. I would expect statements following a declaration
to be almost as common as statements alone, especially in C where
this style of initializing local variables is idiomatic.
>> (I realize this might be outside the scope of the feature request
>> and starting to creep into the -Wunreachable-code territory.)
>
> This really sounds like the old -Wunreachable stuff and I don't think
> it's limited to switches as this warning is. Nowadays we have stuff
> like gimple_stmt_may_fallthru so maybe that could be useful, but I'm
> not about to plunge into this mess anytime soon ;).
Understood. It was just an observation that although ideally
the warning would make use of optimizations like VRP, it seems
that it could be made more useful even without it if it could
be taught to diagnose statements after unconditional breaks.
But I'm sure even that wouldn't be trivial.
Martin
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: C PATCH to add -Wswitch-unreachable (PR c/49859)
2016-05-26 8:27 ` Martin Sebor
@ 2016-05-26 9:32 ` Marek Polacek
2016-05-26 14:39 ` Jason Merrill
0 siblings, 1 reply; 15+ messages in thread
From: Marek Polacek @ 2016-05-26 9:32 UTC (permalink / raw)
To: Martin Sebor; +Cc: Jason Merrill, GCC Patches, Joseph Myers
On Wed, May 25, 2016 at 03:21:00PM -0600, Martin Sebor wrote:
> I see. Thanks for clarifying that. No warning on a declaration
> alone makes sense in the case above but it has the unfortunate
> effect of suppressing the warning when the declaration is followed
> by a statement, such as in:
>
> void f (int*, int);
>
> void g (int i)
> {
> switch (i) {
> int a [3];
> memset (a, 0, sizeof a);
>
> default:
> f (a, 3);
> }
> }
Ah, then I think we should probably look into GIMPLE_TRY, using
gimple_try_eval, too.
Marek
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: C PATCH to add -Wswitch-unreachable (PR c/49859)
2016-05-26 9:32 ` Marek Polacek
@ 2016-05-26 14:39 ` Jason Merrill
0 siblings, 0 replies; 15+ messages in thread
From: Jason Merrill @ 2016-05-26 14:39 UTC (permalink / raw)
To: Marek Polacek; +Cc: Martin Sebor, GCC Patches, Joseph Myers
On Thu, May 26, 2016 at 3:06 AM, Marek Polacek <polacek@redhat.com> wrote:
> On Wed, May 25, 2016 at 03:21:00PM -0600, Martin Sebor wrote:
>> I see. Thanks for clarifying that. No warning on a declaration
>> alone makes sense in the case above but it has the unfortunate
>> effect of suppressing the warning when the declaration is followed
>> by a statement, such as in:
>>
>> void f (int*, int);
>>
>> void g (int i)
>> {
>> switch (i) {
>> int a [3];
>> memset (a, 0, sizeof a);
>>
>> default:
>> f (a, 3);
>> }
>> }
>
> Ah, then I think we should probably look into GIMPLE_TRY, using
> gimple_try_eval, too.
It might also make sense to distinguish between gimple_try_kind of
GIMPLE_TRY_CATCH (a user-written try block) and GIMPLE_TRY_FINALLY (a
compiler-generated cleanup).
Jason
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: C PATCH to add -Wswitch-unreachable (PR c/49859)
2016-05-23 10:17 ` Marek Polacek
@ 2016-05-30 12:33 ` Florian Weimer
0 siblings, 0 replies; 15+ messages in thread
From: Florian Weimer @ 2016-05-30 12:33 UTC (permalink / raw)
To: Marek Polacek; +Cc: Jason Merrill, GCC Patches, Joseph Myers
On 05/23/2016 12:16 PM, Marek Polacek wrote:
>> Can we document what a programmer needs to do to suppress the warning? My
>> worry is that the warning will be too unpredictable to be useful.
>
> Why would you suppress the warning? Just delete the code that cannot be
> executed anyway. And what is unpredictable? You don't want the compiler
> to warn on
> switch(x) {
> found = 1;
> case 1:
> ...
> ?
Oh, I misread what the patch was about. It looks sufficiently
predictable to do this the GIMPLE level (but it's odd if that's indeed
the architecturally correct choice).
Thanks,
Florian
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-05-30 10:08 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-10 18:19 C PATCH to add -Wswitch-unreachable (PR c/49859) Marek Polacek
2016-05-19 15:36 ` Marek Polacek
2016-05-19 15:54 ` Jason Merrill
2016-05-20 16:36 ` Marek Polacek
2016-05-20 16:53 ` Jason Merrill
2016-05-21 2:17 ` Sandra Loosemore
2016-05-23 9:55 ` Marek Polacek
2016-05-23 9:43 ` Florian Weimer
2016-05-23 10:17 ` Marek Polacek
2016-05-30 12:33 ` Florian Weimer
2016-05-23 23:53 ` Martin Sebor
2016-05-25 17:20 ` Marek Polacek
2016-05-26 8:27 ` Martin Sebor
2016-05-26 9:32 ` Marek Polacek
2016-05-26 14:39 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).