| Message ID | 20160520163634.GN1611@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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
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
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
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
>> 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
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
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
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
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;
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. Marek