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.

diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index fbbc924243d3e2d452d8d76ab7fa89fd572f8e6c..3a9ce248e656b424c04d3e0664efcda86988f9be 100644 (file)
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,12 @@
+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.

diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index 6dbc660ddb442c58513caf438ec02137cad9ea83..d8f0ad654fe8c687d15c410805e769dbe32798ff 100644 (file)
--- a/gcc/c-family/c-warn.c
+++ b/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

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index aa0fa5deae6e997447cfe4069e7e6517352a492c..814ed17f7c4aba80368ad8b8303c13759703bd63 100644 (file)
--- a/gcc/c-family/c.opt
+++ b/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.

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index bbd03f87c6750167a7246d45bcdaaf72e8cd0b73..2cd8d7ec5ff7f0b3adf5569a8a95ab24460a12ad 100644 (file)
--- a/gcc/doc/invoke.texi
+++ b/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

diff --git a/gcc/testsuite/c-c++-common/Wconversion-pr40752.c b/gcc/testsuite/c-c++-common/Wconversion-pr40752.c
new file mode 100644 (file)
index 0000000..dc75718
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wconversion-pr40752.c
@@ -0,0 +1,49 @@
+/* { 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" } */
+}

diff --git a/gcc/testsuite/c-c++-common/Wconversion-pr40752a.c b/gcc/testsuite/c-c++-common/Wconversion-pr40752a.c
new file mode 100644 (file)
index 0000000..7b5c9de
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wconversion-pr40752a.c
@@ -0,0 +1,49 @@
+/* { 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" } */
+}

diff --git a/gcc/testsuite/c-c++-common/Wsign-conversion-1.c b/gcc/testsuite/c-c++-common/Wsign-conversion-1.c
new file mode 100644 (file)
index 0000000..20b6e99
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wsign-conversion-1.c
@@ -0,0 +1,13 @@
+/* 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;
+}