This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] c++: Fix spurious fallthrough warning on break
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Siddhesh Poyarekar <siddhesh at sourceware dot org>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Tue, 20 Feb 2018 10:44:47 +0100
- Subject: Re: [PATCH] c++: Fix spurious fallthrough warning on break
- Authentication-results: sourceware.org; auth=none
- References: <20180219162900.31733-1-siddhesh@sourceware.org>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
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