PR c++/40752 - useless -Wconversion with short +=.

This is a longstanding issue with lots of duplicates; people are not
interested in a -Wconversion warning about mychar += 1.  So now that warning
depends on -Warith-conversion; otherwise we only warn if operands of the
arithmetic have conversion issues.

	* c.opt (-Warith-conversion): New.
	* c-warn.c (conversion_warning): Recurse for operands of
	operators.  Only warn about the whole expression with
	-Warith-conversion.

gcc/c-family/ChangeLog

 2020-01-21  Jason Merrill  <jason@redhat.com>
+	    Manuel López-Ibáñez  <manu@gcc.gnu.org>
+
+	PR c++/40752 - useless -Wconversion with short +=.
+	* c.opt (-Warith-conversion): New.
+	* c-warn.c (conversion_warning): Recurse for operands of
+	operators.  Only warn about the whole expression with
+	-Warith-conversion.
+
+2020-01-21  Jason Merrill  <jason@redhat.com>
 
 	* c-common.c (unsafe_conversion_p): Don't warn, return UNSAFE_SIGN.
 	* c-warn.c (conversion_warning): Warn about UNSAFE_SIGN.

gcc/c-family/c-warn.c

@@ -1155,17 +1155,18 @@ check_main_parameter_types (tree decl)
 	     "%q+D declared as variadic function", decl);
 }
 
-/* Warns if the conversion of EXPR to TYPE may alter a value.
+/* Warns and returns true if the conversion of EXPR to TYPE may alter a value.
    This is a helper function for warnings_for_convert_and_check.  */
 
-static void
+static bool
 conversion_warning (location_t loc, tree type, tree expr, tree result)
 {
   tree expr_type = TREE_TYPE (expr);
   enum conversion_safety conversion_kind;
+  bool is_arith = false;
 
   if (!warn_conversion && !warn_sign_conversion && !warn_float_conversion)
-    return;
+    return false;
 
   /* This may happen, because for LHS op= RHS we preevaluate
      RHS and create C_MAYBE_CONST_EXPR <SAVE_EXPR <RHS>>, which
@@ -1195,7 +1196,7 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
       if (TYPE_PRECISION (type) == 1 && !TYPE_UNSIGNED (type))
 	warning_at (loc, OPT_Wconversion,
 		    "conversion to %qT from boolean expression", type);
-      return;
+      return true;
 
     case REAL_CST:
     case INTEGER_CST:
@@ -1250,8 +1251,48 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
 	  warning_at (loc, warnopt,
 		      "conversion from %qT to %qT changes the value of %qE",
 		      expr_type, type, expr);
-	break;
+	return true;
       }
+
+    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);
+	if (conversion_warning (loc, type, op0, result)
+	    || conversion_warning (loc, type, op1, result))
+	  return true;
+	goto arith_op;
+      }
+
+    case PREDECREMENT_EXPR:
+    case PREINCREMENT_EXPR:
+    case POSTDECREMENT_EXPR:
+    case POSTINCREMENT_EXPR:
+    case LSHIFT_EXPR:
+    case RSHIFT_EXPR:
+    case FIX_TRUNC_EXPR:
+    case NON_LVALUE_EXPR:
+    case NEGATE_EXPR:
+    case BIT_NOT_EXPR:
+      {
+	/* Unary ops or binary ops for which we only care about the lhs.  */
+	tree op0 = TREE_OPERAND (expr, 0);
+	if (conversion_warning (loc, type, op0, result))
+	  return true;
+	goto arith_op;
+      }
+
     case COND_EXPR:
       {
 	/* In case of COND_EXPR, we do not care about the type of
@@ -1259,12 +1300,16 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
 	tree op1 = TREE_OPERAND (expr, 1);
 	tree op2 = TREE_OPERAND (expr, 2);
 
-	conversion_warning (loc, type, op1, result);
-	conversion_warning (loc, type, op2, result);
-	return;
+	return (conversion_warning (loc, type, op1, result)
+		|| conversion_warning (loc, type, op2, result));
       }
 
-    default: /* 'expr' is not a constant.  */
+    arith_op:
+      /* We didn't warn about the operands, we might still want to warn if
+	 -Warith-conversion.  */
+      is_arith = true;
+      gcc_fallthrough ();
+    default:
       conversion_kind = unsafe_conversion_p (loc, type, expr, result, true);
       {
 	int warnopt;
@@ -1276,6 +1321,11 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
 	  warnopt = OPT_Wconversion;
 	else
 	  break;
+	if (is_arith
+	    && global_dc->option_enabled (warnopt,
+					  global_dc->lang_mask,
+					  global_dc->option_state))
+	  warnopt = OPT_Warith_conversion;
 	if (conversion_kind == UNSAFE_SIGN)
 	  warning_at (loc, warnopt, "conversion to %qT from %qT "
 		      "may change the sign of the result",
@@ -1288,8 +1338,10 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
 	  warning_at (loc, warnopt,
 		      "conversion from %qT to %qT may change value",
 		      expr_type, type);
+	return true;
       }
     }
+  return false;
 }
 
 /* Produce warnings after a conversion. RESULT is the result of

gcc/c-family/c.opt

@@ -1107,6 +1107,10 @@ Wshift-negative-value
 C ObjC C++ ObjC++ Var(warn_shift_negative_value) Init(-1) Warning
 Warn if left shifting a negative value.
 
+Warith-conversion
+C ObjC C++ ObjC++ Var(warn_arith_conv) Warning
+Warn if conversion of the result of arithmetic might change the value even though converting the operands cannot.
+
 Wsign-compare
 C ObjC C++ ObjC++ Var(warn_sign_compare) Warning LangEnabledBy(C++ ObjC++,Wall)
 Warn about signed-unsigned comparisons.

gcc/doc/invoke.texi

@@ -312,6 +312,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wno-analyzer-use-of-pointer-in-stale-stack-frame @gol
 -Wno-analyzer-use-of-uninitialized-value @gol
 -Wanalyzer-too-complex @gol
+-Warith-conversion @gol
 -Warray-bounds  -Warray-bounds=@var{n} @gol
 -Wno-attributes  -Wattribute-alias=@var{n}  @gol
 -Wbool-compare  -Wbool-operation @gol
@@ -6637,6 +6638,24 @@ This warning requires @option{-fanalyzer}, which enables it; use
 This diagnostic warns for paths through the code in which an uninitialized
 value is used.
 
+@item -Warith-conversion
+@opindex Warith-conversion
+@opindex Wno-arith-conversion
+Do warn about implicit conversions from arithmetic operations even
+when conversion of the operands to the same type cannot change their
+values.  This affects warnings from @option{-Wconversion},
+@option{-Wfloat-conversion}, and @option{-Wsign-conversion}.
+
+@smallexample
+@group
+void f (char c, int i)
+@{
+  c = c + i; // warns with @option{-Wconversion}
+  c = c + 1; // only warns with @option{-Warith-conversion}
+@}
+@end group
+@end smallexample
+
 @item -Warray-bounds
 @itemx -Warray-bounds=@var{n}
 @opindex Wno-array-bounds
@@ -7425,6 +7444,9 @@ reference to them. Warnings about conversions between signed and
 unsigned integers are disabled by default in C++ unless
 @option{-Wsign-conversion} is explicitly enabled.
 
+Warnings about conversion from arithmetic on a small type back to that
+type are only given with @option{-Warith-conversion}.
+
 @item -Wno-conversion-null @r{(C++ and Objective-C++ only)}
 @opindex Wconversion-null
 @opindex Wno-conversion-null

gcc/testsuite/c-c++-common/Wconversion-pr40752.c

+/* { 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 *= 2;
+  c *= c2;
+  c /= 2;
+  c /= c2;
+  c %= 2;
+  c %= c2;
+  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" } */
+}

gcc/testsuite/c-c++-common/Wconversion-pr40752a.c

+/* { dg-do compile } */
+/* { dg-options "-Wconversion -Warith-conversion" } */
+#include <limits.h>
+void foo(char c, char c2)
+{
+  c >>= c2;			/* { dg-warning "conversion" } */
+  c >>= 1;
+  c <<= 1;			/* { dg-warning "conversion" } */
+  c <<= c2;			/* { dg-warning "conversion" } */
+  c += 1;			/* { dg-warning "conversion" } */
+  c += c2;			/* { dg-warning "conversion" } */
+  c -= 1;			/* { dg-warning "conversion" } */
+  c -= c2;			/* { dg-warning "conversion" } */
+  c *= 2;			/* { dg-warning "conversion" } */
+  c *= c2;			/* { dg-warning "conversion" } */
+  c /= 2;
+  c /= c2;			/* { dg-warning "conversion" } */
+  c %= 2;
+  c %= c2;			/* { dg-warning "conversion" } */
+  c = -c2;			/* { dg-warning "conversion" } */
+  c = ~c2;			/* { dg-warning "conversion" } */
+  c = c2++;
+  c = ++c2;
+  c = c2--;
+  c = --c2;
+}
+
+void bar(char c, int c2)
+{
+  c >>= c2; 			/* { dg-warning "conversion" } */
+  c >>= (int)1;
+  c <<= (int)1;			/* { 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 %= ((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" } */
+}

gcc/testsuite/c-c++-common/Wsign-conversion-1.c

+/* PR c++/52703 */
+/* { dg-options -Wsign-conversion } */
+
+unsigned f (unsigned x) {
+  return x;
+}
+
+int main () {
+  unsigned short a = 0;
+  unsigned b = a + 1;
+  f (a + 1);
+  return 0;
+}