From c77074d05691053ee7347d9e44ab89b3adb23fb1 Mon Sep 17 00:00:00 2001 From: Jason Merrill Date: Tue, 7 Jan 2020 12:20:26 -0500 Subject: [PATCH] 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 | 9 +++ gcc/c-family/c-warn.c | 70 ++++++++++++++++--- gcc/c-family/c.opt | 4 ++ gcc/doc/invoke.texi | 22 ++++++ .../c-c++-common/Wconversion-pr40752.c | 49 +++++++++++++ .../c-c++-common/Wconversion-pr40752a.c | 49 +++++++++++++ .../c-c++-common/Wsign-conversion-1.c | 13 ++++ 7 files changed, 207 insertions(+), 9 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/Wconversion-pr40752.c create mode 100644 gcc/testsuite/c-c++-common/Wconversion-pr40752a.c create mode 100644 gcc/testsuite/c-c++-common/Wsign-conversion-1.c diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog index fbbc924243d..3a9ce248e65 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,12 @@ +2020-01-21 Jason Merrill + Manuel López-Ibáñez + + 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 * 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 6dbc660ddb4..d8f0ad654fe 100644 --- 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 >, 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 aa0fa5deae6..814ed17f7c4 100644 --- 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 bbd03f87c67..2cd8d7ec5ff 100644 --- 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 index 00000000000..dc757185c75 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wconversion-pr40752.c @@ -0,0 +1,49 @@ +/* { dg-do compile } */ +/* { dg-options "-Wconversion" } */ +#include +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 index 00000000000..7b5c9dee617 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wconversion-pr40752a.c @@ -0,0 +1,49 @@ +/* { dg-do compile } */ +/* { dg-options "-Wconversion -Warith-conversion" } */ +#include +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 index 00000000000..20b6e999b2f --- /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; +} -- 2.30.2