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

2020-01-30 Thread Joseph Myers
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.

-- 
Joseph S. Myers
jos...@codesourcery.com


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

2020-01-29 Thread Jeff Law

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.
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 000..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;
+   }
+}
+
+