diff mbox series

[RFA,c/88660] Fix bogus set-but-unused warning

Message ID 0f8ec6bad2e2a182126b8c3c6f1ec2b560ec3e6a.camel@redhat.com
State New
Headers show
Series [RFA,c/88660] Fix bogus set-but-unused warning | expand

Commit Message

Jeff Law Jan. 30, 2020, 2:32 a.m. UTC 
Joseph, you were the last one in this part of
c_parser_switch_statement, but the change you made was for supporting
atomics.  I'm not sure if you'll have context here or not.

PR88660 is a false positive warning for a set-but-unused object.  We
can see trivially from the testcase that "i" is used in the switch
expression, but we get a set-but-unused warning.

In the last major change in this code was ~5 years ago and twiddled the
handling of the switch expression to call convert_lvalue_to_rvalue.

The last argument to that function indicates whether or not we should
mark the switch expression as a use of the object.  We're currently
passing in "false" so the object doesn't get marked and we get the
bogus warning.

The obvious fix is to pass in "true", which is what the proposed patch
does.  If there's a reason we can't/shouldn't do that in this case we
could call mark_exp_read on the switch expression at some point other
point.

Bootstrapped and regression tested on x86_64.  OK for the trunk?

Jeff
Mark switch expression as used to avoid bogus warning

	PR c/88660
	* c-parser.c (c_parser_switch_statement): Make sure to request
	marking the switch expr as used.

	PR c/88660
	* gcc.dg/pr88660.c: New test.

Comments

Joseph Myers Jan. 30, 2020, 5:33 p.m. UTC | #1 
On Wed, 29 Jan 2020, Jeff Law wrote:

> In the last major change in this code was ~5 years ago and twiddled the
> handling of the switch expression to call convert_lvalue_to_rvalue.
> 
> The last argument to that function indicates whether or not we should
> mark the switch expression as a use of the object.  We're currently
> passing in "false" so the object doesn't get marked and we get the
> bogus warning.
> 
> The obvious fix is to pass in "true", which is what the proposed patch
> does.  If there's a reason we can't/shouldn't do that in this case we
> could call mark_exp_read on the switch expression at some point other
> point.
> 
> Bootstrapped and regression tested on x86_64.  OK for the trunk?

I think the code passes false simply because it was a straight conversion 
from previous code that (probably erroneously) didn't call mark_exp_read.  
The patch is OK.
diff mbox series

Patch

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 6164017de02..1e8f2f7108d 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -6607,7 +6607,7 @@  c_parser_switch_statement (c_parser *parser, bool *if_p)
 	  && c_token_starts_typename (c_parser_peek_2nd_token (parser)))
 	explicit_cast_p = true;
       ce = c_parser_expression (parser);
-      ce = convert_lvalue_to_rvalue (switch_cond_loc, ce, true, false);
+      ce = convert_lvalue_to_rvalue (switch_cond_loc, ce, true, true);
       expr = ce.value;
       /* ??? expr has no valid location?  */
       parens.skip_until_found_close (parser);
diff --git a/gcc/testsuite/gcc.dg/pr88660.c b/gcc/testsuite/gcc.dg/pr88660.c
new file mode 100644
index 00000000000..09a2d3270bd
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr88660.c
@@ -0,0 +1,13 @@ 
+/* { dg-do-compile } */
+/* { dg-options "-O -Wunused-but-set-variable" } */
+
+int main(void)
+{
+	const int i = 0;
+	switch(i)
+	{
+	default: break;
+	}
+}
+
+