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
