Re: PR 40752: -Wconversion generates false warnings for operands not larger than target type

[Manuel López-Ibáñez <lopezibanez at gmail dot com>]

Modified the patch to make use of the new c-c++-common testsuite.
Bootstrapped and regression tested on x86_64-linux-gnu.

OK for trunk?

2009-08-11  Manuel López-Ibáñez  <manu@gcc.gnu.org>

	PR 40752
	* c-common.c (conversion_warning): Strip useless
	conversions. Recurse for operands of operators. Warn at expression
	location.
testsuite/
	* c-c++-common/Wconversion-pr40752.c: New.

2009/7/22 Manuel López-Ibáñez <lopezibanez@gmail.com>:
> Bootstrapped and regression tested on x86_64-linux-gnu.
>
> OK for trunk?
>
> 2009-07-22 ?Manuel López-Ibáñez ?<manu@gcc.gnu.org>
>
> ? ? ? ?PR 40752
> ? ? ? ?* c-common.c (conversion_warning): Strip useless
> ? ? ? ?conversions. Recurse for operands of operators. Warn at expression
> ? ? ? ?location.
> testsuite/
> ? ? ? ?* gcc.dg/Wconversion-pr40752.c: New.
>

Index: gcc/testsuite/c-c++-common/Wconversion-pr40752.c
===================================================================
--- gcc/testsuite/c-c++-common/Wconversion-pr40752.c	(revision 0)
+++ gcc/testsuite/c-c++-common/Wconversion-pr40752.c	(revision 0)
@@ -0,0 +1,48 @@
+/* { dg-do compile } */
+/* { dg-options "-Wconversion" } */
+#include <limits.h>
+void foo(char c, char c2)
+{
+  c >>= c2;
+  c >>= 1;
+  c <<= 1;
+  c <<= c2;
+  c += 1;
+  c += c2;
+  c -= 1;
+  c -= c2;
+  c *= 1;
+  c *= c2;
+  c /= 1;
+  c /= c2;
+  c %= 1;
+  c %= c2;
+  c = ~c2;
+  c = c2++;
+  c = ++c2;
+  c = c2--;
+  c = --c2;
+}
+
+void bar(char c, int c2)
+{
+  c >>= c2; 
+  c >>= (int)1;
+  c <<= (int)1;
+  c <<= c2;
+  c += ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */
+  c += c2; /* { dg-warning "conversion" } */
+  c -= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */
+  c -= c2; /* { dg-warning "conversion" } */
+  c *= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */
+  c *= c2; /* { dg-warning "conversion" } */
+  c /= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */
+  c /= c2; /* { dg-warning "conversion" } */
+  c %= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */
+  c %= c2; /* { dg-warning "conversion" } */
+  c = ~c2; /* { dg-warning "conversion" } */
+  c = c2++; /* { dg-warning "conversion" } */
+  c = ++c2; /* { dg-warning "conversion" } */
+  c = c2--; /* { dg-warning "conversion" } */
+  c = --c2; /* { dg-warning "conversion" } */
+}
Index: gcc/c-common.c
===================================================================
--- gcc/c-common.c	(revision 150627)
+++ gcc/c-common.c	(working copy)
@@ -1972,11 +1972,13 @@ conversion_warning (tree type, tree expr
 {
   bool give_warning = false;
 
   int i;
   const int expr_num_operands = TREE_OPERAND_LENGTH (expr);
-  tree expr_type = TREE_TYPE (expr);
+  tree expr_type;
+  location_t loc = EXPR_HAS_LOCATION (expr) 
+    ? EXPR_LOCATION (expr) : input_location;
 
   if (!warn_conversion && !warn_sign_conversion)
     return;
 
   /* If any operand is artificial, then this expression was generated
@@ -1986,10 +1988,15 @@ conversion_warning (tree type, tree expr
       tree op = TREE_OPERAND (expr, i);
       if (op && DECL_P (op) && DECL_ARTIFICIAL (op))
 	return;
     }
 
+  STRIP_USELESS_TYPE_CONVERSION (expr);
+  /* Don't warn about unsigned char y = 0xff, x = (int) y;  */
+  expr = get_unwidened (expr, 0);
+  expr_type = TREE_TYPE (expr);
+
   switch (TREE_CODE (expr))
     {
     case EQ_EXPR:
     case NE_EXPR:
     case LE_EXPR:
@@ -2004,12 +2011,12 @@ conversion_warning (tree type, tree expr
     case TRUTH_NOT_EXPR:
       /* Conversion from boolean to a signed:1 bit-field (which only
 	 can hold the values 0 and -1) doesn't lose information - but
 	 it does change the value.  */
       if (TYPE_PRECISION (type) == 1 && !TYPE_UNSIGNED (type)) 
-	warning (OPT_Wconversion,
-                 "conversion to %qT from boolean expression", type);
+	warning_at (loc, OPT_Wconversion,
+		    "conversion to %qT from boolean expression", type);
       return;
 
     case REAL_CST:
     case INTEGER_CST:
 
@@ -2026,15 +2033,15 @@ conversion_warning (tree type, tree expr
                && TREE_CODE (type) == INTEGER_TYPE
                && !int_fits_type_p (expr, type))
         {
           if (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (expr_type) 
 	      && tree_int_cst_sgn (expr) < 0)
-	    warning (OPT_Wsign_conversion,
-		     "negative integer implicitly converted to unsigned type");
+	    warning_at (loc, OPT_Wsign_conversion, "negative integer"
+			" implicitly converted to unsigned type");
           else if (!TYPE_UNSIGNED (type) && TYPE_UNSIGNED (expr_type))
-	    warning (OPT_Wsign_conversion,  "conversion of unsigned constant "
-		     "value to negative integer");
+	    warning_at (loc, OPT_Wsign_conversion, "conversion of unsigned"
+			" constant value to negative integer");
 	  else
 	    give_warning = true;
         }
       else if (TREE_CODE (type) == REAL_TYPE)
         {
@@ -2055,34 +2062,68 @@ conversion_warning (tree type, tree expr
                 give_warning = true;
             }
         }
 
       if (give_warning)
-        warning (OPT_Wconversion,
-                 "conversion to %qT alters %qT constant value",
-                 type, expr_type);
+        warning_at (loc, OPT_Wconversion,
+		    "conversion to %qT alters %qT constant value",
+		    type, expr_type);
 
       return;
 
+    case PLUS_EXPR:
+    case MINUS_EXPR:
+    case MULT_EXPR:
+    case MAX_EXPR:
+    case MIN_EXPR:
+    case TRUNC_MOD_EXPR:
+    case FLOOR_MOD_EXPR:
+    case TRUNC_DIV_EXPR:
+    case FLOOR_DIV_EXPR:
+    case CEIL_DIV_EXPR:
+    case EXACT_DIV_EXPR:
+    case RDIV_EXPR:
+      {
+	tree op0 = TREE_OPERAND (expr, 0);
+	tree op1 = TREE_OPERAND (expr, 1);
+	conversion_warning (type, op0);
+	conversion_warning (type, op1);
+	return;
+      }
+    case PREDECREMENT_EXPR:
+    case PREINCREMENT_EXPR:
+    case POSTDECREMENT_EXPR:
+    case POSTINCREMENT_EXPR:
+    case LSHIFT_EXPR:
+    case RSHIFT_EXPR:
+      {
+	/* These are binary, but we only care about the lhs operand.  */
+	tree op0 = TREE_OPERAND (expr, 0);
+	conversion_warning (type, op0);
+	return;
+      }
+
+    case FIX_TRUNC_EXPR:
+    case NON_LVALUE_EXPR:
+    case NEGATE_EXPR:
+    case BIT_NOT_EXPR:
+      {
+      /* Unary operations.  */
+	tree op0 = TREE_OPERAND (expr, 0);
+	conversion_warning (type, op0);
+	return;
+      }
     case COND_EXPR:
       {
-	/* In case of COND_EXPR, if both operands are constants or
-	   COND_EXPR, then we do not care about the type of COND_EXPR,
-	   only about the conversion of each operand.  */
+	/* In case of COND_EXPR, we do not care about the type of
+	   COND_EXPR, only about the conversion of each operand.  */
 	tree op1 = TREE_OPERAND (expr, 1);
 	tree op2 = TREE_OPERAND (expr, 2);
 
-	if ((TREE_CODE (op1) == REAL_CST || TREE_CODE (op1) == INTEGER_CST 
-	     || TREE_CODE (op1) == COND_EXPR)
-	    && (TREE_CODE (op2) == REAL_CST || TREE_CODE (op2) == INTEGER_CST
-		|| TREE_CODE (op2) == COND_EXPR))
-	  {
-	    conversion_warning (type, op1);
-	    conversion_warning (type, op2);
-	    return;
-	  }
-	/* Fall through.  */
+	conversion_warning (type, op1);
+	conversion_warning (type, op2);
+	return;
       }
 
     default: /* 'expr' is not a constant.  */
 
       /* Warn for real types converted to integer types.  */
@@ -2091,14 +2132,10 @@ conversion_warning (tree type, tree expr
         give_warning = true;
 
       else if (TREE_CODE (expr_type) == INTEGER_TYPE
                && TREE_CODE (type) == INTEGER_TYPE)
         {
-	  /* Don't warn about unsigned char y = 0xff, x = (int) y;  */
-	  expr = get_unwidened (expr, 0);
-	  expr_type = TREE_TYPE (expr);
-
 	  /* Don't warn for short y; short x = ((int)y & 0xff);  */
 	  if (TREE_CODE (expr) == BIT_AND_EXPR 
 		|| TREE_CODE (expr) == BIT_IOR_EXPR 
 	      || TREE_CODE (expr) == BIT_XOR_EXPR)
 	    {
@@ -2148,13 +2185,13 @@ conversion_warning (tree type, tree expr
 		    && TYPE_UNSIGNED (expr_type) != TYPE_UNSIGNED (type))
 		   /* Even when converted to a bigger type, if the type is
 		      unsigned but expr is signed, then negative values
 		      will be changed.  */
 		   || (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (expr_type)))
-	    warning (OPT_Wsign_conversion, "conversion to %qT from %qT "
-		     "may change the sign of the result",
-		     type, expr_type);
+	    warning_at (loc, OPT_Wsign_conversion, "conversion to %qT from %qT "
+			"may change the sign of the result",
+			type, expr_type);
         }
 
       /* Warn for integer types converted to real types if and only if
          all the range of values of the integer type cannot be
          represented by the real type.  */
@@ -2179,13 +2216,13 @@ conversion_warning (tree type, tree expr
                && TYPE_PRECISION (type) < TYPE_PRECISION (expr_type))
         give_warning = true;
 
 
       if (give_warning)
-        warning (OPT_Wconversion,
-                 "conversion to %qT from %qT may alter its value",
-                 type, expr_type);
+        warning_at (loc, OPT_Wconversion,
+		    "conversion to %qT from %qT may alter its value",
+		    type, expr_type);
     }
 }
 
 /* Produce warnings after a conversion. RESULT is the result of
    converting EXPR to TYPE.  This is a helper function for