Re: [PATCH] c++: Fix spurious fallthrough warning on break
On Tuesday 20 February 2018 03:14 PM, Jakub Jelinek wrote: > On Mon, Feb 19, 2018 at 09:59:00PM +0530, Siddhesh Poyarekar wrote: >> The C++ frontend generates a break that results in the fallthrough >> warning misfiring in nested switch blocks where cases in the inner >> switch block return, rendering the break pointless. The fallthrough >> detection in finish_break_stmt does not work either because the >> condition is encoded as an IF_STMT and not a COND_EXPR. >> >> Fix this by adding a condition for IF_STMT in the >> langhooks.block_may_fallthru for C++. Fix tested on x86_64. Full >> testsuite run is in progress. >> >> Siddhesh >> >> gcc/cp >> * cp-objcp-common.c (cxx_block_may_fallthru): Add case for >> IF_STMT. >> >> gcc/testsuite >> * g++.dg/nested-switch.C: New test case. > > C++ testcase shouldn't be put directly into g++.dg/ (though yes, several > have been), but into a subdirectory. In this case, because it is a (bogus) > warning, it should best go into g++.dg/warn/, and perhaps best named as > g++.dg/warn/Wimplicit-fallthrough-3.C . > >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/nested-switch.C >> @@ -0,0 +1,30 @@ >> +// Verify that there are no spurious warnings in nested switch statements >> due >> +// to the unnecessary break in the inner switch block. >> +// { dg-do compile } >> +// { dg-options "-Werror=implicit-fallthrough" } */ > > I'd go just for -Wimplicit-fallthrough. > >> + >> +int >> +foo (int c1, int c2, int c3) >> +{ >> + switch (c2) >> +{ >> +case 0: >> + switch (c3) { > > And turn the above line into > switch (c3) { // { dg-bogus "may fall through" } > >> +case 0: >> + if (c1) >> +return 1; >> + else >> +return 2; >> + break; >> + >> +default: >> + return 3; >> + } >> + >> +case 1: >> + return 4; >> +default: >> + return 5; >> + break; >> +} >> +} > > Ok for trunk with those nits, thanks. Thanks, fixed up and pushed. Siddhesh
Re: [PATCH] c++: Fix spurious fallthrough warning on break
On Mon, Feb 19, 2018 at 09:59:00PM +0530, Siddhesh Poyarekar wrote: > The C++ frontend generates a break that results in the fallthrough > warning misfiring in nested switch blocks where cases in the inner > switch block return, rendering the break pointless. The fallthrough > detection in finish_break_stmt does not work either because the > condition is encoded as an IF_STMT and not a COND_EXPR. > > Fix this by adding a condition for IF_STMT in the > langhooks.block_may_fallthru for C++. Fix tested on x86_64. Full > testsuite run is in progress. > > Siddhesh > > gcc/cp > * cp-objcp-common.c (cxx_block_may_fallthru): Add case for > IF_STMT. > > gcc/testsuite > * g++.dg/nested-switch.C: New test case. C++ testcase shouldn't be put directly into g++.dg/ (though yes, several have been), but into a subdirectory. In this case, because it is a (bogus) warning, it should best go into g++.dg/warn/, and perhaps best named as g++.dg/warn/Wimplicit-fallthrough-3.C . > --- /dev/null > +++ b/gcc/testsuite/g++.dg/nested-switch.C > @@ -0,0 +1,30 @@ > +// Verify that there are no spurious warnings in nested switch statements due > +// to the unnecessary break in the inner switch block. > +// { dg-do compile } > +// { dg-options "-Werror=implicit-fallthrough" } */ I'd go just for -Wimplicit-fallthrough. > + > +int > +foo (int c1, int c2, int c3) > +{ > + switch (c2) > +{ > +case 0: > + switch (c3) { And turn the above line into switch (c3) { // { dg-bogus "may fall through" } > + case 0: > + if (c1) > + return 1; > + else > + return 2; > + break; > + > + default: > + return 3; > + } > + > +case 1: > + return 4; > +default: > + return 5; > + break; > +} > +} Ok for trunk with those nits, thanks. Jakub
Re: [PATCH] c++: Fix spurious fallthrough warning on break
On Monday 19 February 2018 09:59 PM, Siddhesh Poyarekar wrote: > The C++ frontend generates a break that results in the fallthrough > warning misfiring in nested switch blocks where cases in the inner > switch block return, rendering the break pointless. The fallthrough > detection in finish_break_stmt does not work either because the > condition is encoded as an IF_STMT and not a COND_EXPR. > > Fix this by adding a condition for IF_STMT in the > langhooks.block_may_fallthru for C++. Fix tested on x86_64. Full > testsuite run is in progress. Now confirmed that the patch does not introduce any new regressions. Siddhesh
[PATCH] c++: Fix spurious fallthrough warning on break
The C++ frontend generates a break that results in the fallthrough warning misfiring in nested switch blocks where cases in the inner switch block return, rendering the break pointless. The fallthrough detection in finish_break_stmt does not work either because the condition is encoded as an IF_STMT and not a COND_EXPR. Fix this by adding a condition for IF_STMT in the langhooks.block_may_fallthru for C++. Fix tested on x86_64. Full testsuite run is in progress. Siddhesh gcc/cp * cp-objcp-common.c (cxx_block_may_fallthru): Add case for IF_STMT. gcc/testsuite * g++.dg/nested-switch.C: New test case. --- gcc/cp/cp-objcp-common.c | 5 + gcc/testsuite/g++.dg/nested-switch.C | 30 ++ 2 files changed, 35 insertions(+) create mode 100644 gcc/testsuite/g++.dg/nested-switch.C diff --git a/gcc/cp/cp-objcp-common.c b/gcc/cp/cp-objcp-common.c index a45dda4d012..5289a89e486 100644 --- a/gcc/cp/cp-objcp-common.c +++ b/gcc/cp/cp-objcp-common.c @@ -349,6 +349,11 @@ cxx_block_may_fallthru (const_tree stmt) case THROW_EXPR: return false; +case IF_STMT: + if (block_may_fallthru (THEN_CLAUSE (stmt))) + return true; + return block_may_fallthru (ELSE_CLAUSE (stmt)); + case SWITCH_STMT: return (!SWITCH_STMT_ALL_CASES_P (stmt) || !SWITCH_STMT_NO_BREAK_P (stmt) diff --git a/gcc/testsuite/g++.dg/nested-switch.C b/gcc/testsuite/g++.dg/nested-switch.C new file mode 100644 index 000..46412dd293f --- /dev/null +++ b/gcc/testsuite/g++.dg/nested-switch.C @@ -0,0 +1,30 @@ +// Verify that there are no spurious warnings in nested switch statements due +// to the unnecessary break in the inner switch block. +// { dg-do compile } +// { dg-options "-Werror=implicit-fallthrough" } */ + +int +foo (int c1, int c2, int c3) +{ + switch (c2) +{ +case 0: + switch (c3) { + case 0: + if (c1) + return 1; + else + return 2; + break; + + default: + return 3; + } + +case 1: + return 4; +default: + return 5; + break; +} +} -- 2.14.3