diff mbox

[C] Get rid of spurious warning (PR c/60036)

Message ID 20140204170205.GM8907@redhat.com
State New
Headers show

Commit Message

Marek Polacek Feb. 4, 2014, 5:02 p.m. UTC 
Starting with revision 206620, if we have LHS op= RHS and RHS has
side effects, we preevaluate RHS first, which means that we wrap it
in C_MAYBE_CONST_EXPR_EXPR + SAVE_EXPR.  This regressed -Wconversion,
since we now issue bogus warnings.  The reason is that conversion_warning
doesn't grok C_MAYBE_CONST_EXPR_EXPR/SAVE_EXPR, meaning that it
just calls unsafe_conversion_p on it that warns.  But for e.g. relational
and equality operators that's useless as those always yield 0 or 1.
This patch fixes this by unwrapping the expr first.

Regtested/bootstrapped on x86_64-linux, ok for trunk?

2014-02-04  Marek Polacek  <polacek@redhat.com>

	PR c/60036
c-family/
	* c-common.c (conversion_warning): Unwrap C_MAYBE_CONST_EXPR and/or
	SAVE_EXPR.
testsuite/
	* gcc.dg/pr60036.c: New test.


	Marek

Comments

Joseph Myers Feb. 4, 2014, 5:18 p.m. UTC | #1 
On Tue, 4 Feb 2014, Marek Polacek wrote:

> Starting with revision 206620, if we have LHS op= RHS and RHS has
> side effects, we preevaluate RHS first, which means that we wrap it
> in C_MAYBE_CONST_EXPR_EXPR + SAVE_EXPR.  This regressed -Wconversion,
> since we now issue bogus warnings.  The reason is that conversion_warning
> doesn't grok C_MAYBE_CONST_EXPR_EXPR/SAVE_EXPR, meaning that it
> just calls unsafe_conversion_p on it that warns.  But for e.g. relational
> and equality operators that's useless as those always yield 0 or 1.
> This patch fixes this by unwrapping the expr first.
> 
> Regtested/bootstrapped on x86_64-linux, ok for trunk?

OK.
diff mbox

Patch

diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index fc12788..007e727 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -2714,6 +2714,14 @@  conversion_warning (location_t loc, tree type, tree expr)
   if (!warn_conversion && !warn_sign_conversion && !warn_float_conversion)
     return;
 
+  /* This may happen, because for LHS op= RHS we preevaluate
+     RHS and create C_MAYBE_CONST_EXPR <SAVE_EXPR <RHS>>, which
+     means we could no longer see the code of the EXPR.  */
+  if (TREE_CODE (expr) == C_MAYBE_CONST_EXPR)
+    expr = C_MAYBE_CONST_EXPR_EXPR (expr);
+  if (TREE_CODE (expr) == SAVE_EXPR)
+    expr = TREE_OPERAND (expr, 0);
+
   switch (TREE_CODE (expr))
     {
     case EQ_EXPR:
diff --git gcc/testsuite/gcc.dg/pr60036.c gcc/testsuite/gcc.dg/pr60036.c
index e69de29..07eb6ac 100644
--- gcc/testsuite/gcc.dg/pr60036.c
+++ gcc/testsuite/gcc.dg/pr60036.c
@@ -0,0 +1,28 @@ 
+/* PR c/60036 */
+/* { dg-do compile } */
+/* { dg-options "-Wconversion" } */
+
+extern int fn (void);
+
+void
+foo (int i)
+{
+  unsigned int f = 9;
+
+  /* Don't warn on these.  */
+  f += fn () || i;
+  f += fn () && i;
+  f += ! fn ();
+  f -= fn () == i;
+  f |= fn () != i;
+  f &= fn () < i;
+  f ^= fn () > i;
+  f &= fn () <= i;
+  f ^= fn () >= i;
+
+  /* But warn on the following.  */
+  f += fn (); /* { dg-warning "conversion" } */
+  f += fn () | i; /* { dg-warning "conversion" } */
+  f += fn () & i; /* { dg-warning "conversion" } */
+  f += fn () ^ i; /* { dg-warning "conversion" } */
+}