Back in 2009 Manuel sent a patch to avoid useless -Wconversion warnings
on compound assignment of types that get promoted to int:
https://gcc.gnu.org/ml/gcc-patches/2009-08/msg00582.html
Joseph argued that those warnings are sometimes useful, and that they
should be controlled by a separate flag. So this patch introduces
-Warith-conversion, which is off by default in this patch.
Joseph, is that default OK with you? If not, can you explain your
reasoning more about why the warnings are desirable? It seems to me
that even in cases where you don't know the ranges involved, it's wrong
for e.g. 'mychar += 1' to warn when 'myint += 1' doesn't. In both
cases, the addition might overflow the range of the target type, but
that isn't about the conversion; the result is the same as if the
operation had been done in the operand/target type rather than the
promoted type.
The change to cp/typeck.c is a placeholder to use if the default setting
changes; even if we end up warning by default for mychar = mychar + 1, I
really don't want to warn about mychar += 1.
sign.diff is a prerequisite tidying that moves the warnings from
unsafe_conversion_p back into conversion_warning with the others.
rshift.diff restores the short_shift code to the C++ front end so that C
and C++ give the same warnings with -Warith-conversion.
Tested x86_64-pc-linux-gnu. Comments?
commit 77b31408e3de8178f20972744d06501adb6ea961
Author: Jason Merrill <ja...@redhat.com>
Date: Tue Jan 7 12:20:26 2020 -0500
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/doc/invoke.texi b/gcc/doc/invoke.texi
index 53df4b1fdf9..d4ab699ccc7 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -291,7 +291,8 @@ Objective-C and Objective-C++ Dialects}.
-Waggregate-return -Waligned-new @gol
-Walloc-zero -Walloc-size-larger-than=@var{byte-size} @gol
-Walloca -Walloca-larger-than=@var{byte-size} @gol
--Wno-aggressive-loop-optimizations -Warray-bounds -Warray-bounds=@var{n} @gol
+-Wno-aggressive-loop-optimizations -Warith-conversion @gol
+-Warray-bounds -Warray-bounds=@var{n} @gol
-Wno-attributes -Wattribute-alias=@var{n} @gol
-Wbool-compare -Wbool-operation @gol
-Wno-builtin-declaration-mismatch @gol
@@ -6244,6 +6245,24 @@ See also @option{-Wvla-larger-than=}@samp{byte-size}.
Disable @option{-Walloca-larger-than=} warnings. The option is
equivalent to @option{-Walloca-larger-than=}@samp{SIZE_MAX} or larger.
+@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
@@ -7032,6 +7051,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/c-family/c.opt b/gcc/c-family/c.opt
index 230b76387ba..88bc4f443e2 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/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 <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/cp/typeck.c b/gcc/cp/typeck.c
index 7b653cebca0..83a70972115 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -8672,8 +8672,11 @@ cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
ICR_INIT, NULL_TREE, 0,
complain);
else
- newrhs = convert_for_assignment (olhstype, newrhs, ICR_ASSIGN,
- NULL_TREE, 0, complain, LOOKUP_IMPLICIT);
+ {
+ //warning_sentinel w (warn_arith_conv, !plain_assign);
+ newrhs = convert_for_assignment (olhstype, newrhs, ICR_ASSIGN,
+ NULL_TREE, 0, complain, LOOKUP_IMPLICIT);
+ }
if (!same_type_p (lhstype, olhstype))
newrhs = cp_convert_and_check (lhstype, newrhs, complain);
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 <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
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 <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
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;
+}
diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index 7acf8a1abcb..f3b2819de1a 100644
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,12 @@
+2020-01-09 Jason Merrill <ja...@redhat.com>
+ Manuel López-Ibáñez <m...@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-10 Jason Merrill <ja...@redhat.com>
* c-common.c (unsafe_conversion_p): Don't warn, return UNSAFE_SIGN.
commit dc64cb4da0d2a7e5ea4440c7c7f9041b7145e14c
Author: Jason Merrill <ja...@redhat.com>
Date: Fri Jan 10 12:49:03 2020 -0500
Handle -Wsign-conversion in conversion_warning.
It seemed strange to me to warn about sign conversion in
unsafe_conversion_p, when other warnings are in conversion_warning, and the
latter function is the only place that asks the former function to warn.
This change is also necessary for my -Warith-conversion patch.
* c-common.c (unsafe_conversion_p): Don't warn, return UNSAFE_SIGN.
* c-warn.c (conversion_warning): Warn about UNSAFE_SIGN.
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 37b0594995a..cb1ae85cbb9 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -779,8 +779,7 @@ enum conversion_safety {
SAFE_CONVERSION = 0,
/* Another type of conversion with problems. */
UNSAFE_OTHER,
- /* Conversion between signed and unsigned integers
- which are all warned about immediately, so this is unused. */
+ /* Conversion between signed and unsigned integers. */
UNSAFE_SIGN,
/* Conversions that reduce the precision of reals including conversions
from reals to integers. */
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index e2d4fd0605a..b63c2999f2b 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -1307,9 +1307,9 @@ int_safely_convertible_to_real_p (const_tree from_type, const_tree to_type)
* EXPR is not a constant of integer type which cannot be
exactly converted to real type.
- Function allows conversions between types of different signedness and
- can return SAFE_CONVERSION (zero) in that case. Function can produce
- signedness warnings if PRODUCE_WARNS is true.
+ Function allows conversions between types of different signedness if
+ CHECK_SIGN is false and can return SAFE_CONVERSION (zero) in that
+ case. Function can return UNSAFE_SIGN if CHECK_SIGN is true.
RESULT, when non-null is the result of the conversion. When constant
it is included in the text of diagnostics.
@@ -1320,14 +1320,11 @@ int_safely_convertible_to_real_p (const_tree from_type, const_tree to_type)
enum conversion_safety
unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
- bool produce_warns)
+ bool check_sign)
{
enum conversion_safety give_warning = SAFE_CONVERSION; /* is 0 or false */
tree expr_type = TREE_TYPE (expr);
- bool cstresult = (result
- && TREE_CODE_CLASS (TREE_CODE (result)) == tcc_constant);
-
loc = expansion_point_location_if_in_system_header (loc);
expr = fold_for_warn (expr);
@@ -1355,32 +1352,13 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
if (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (expr_type)
&& tree_int_cst_sgn (expr) < 0)
{
- if (produce_warns)
- {
- if (cstresult)
- warning_at (loc, OPT_Wsign_conversion,
- "unsigned conversion from %qT to %qT "
- "changes value from %qE to %qE",
- expr_type, type, expr, result);
- else
- warning_at (loc, OPT_Wsign_conversion,
- "unsigned conversion from %qT to %qT "
- "changes the value of %qE",
- expr_type, type, expr);
- }
+ if (check_sign)
+ give_warning = UNSAFE_SIGN;
}
else if (!TYPE_UNSIGNED (type) && TYPE_UNSIGNED (expr_type))
{
- if (cstresult)
- warning_at (loc, OPT_Wsign_conversion,
- "signed conversion from %qT to %qT changes "
- "value from %qE to %qE",
- expr_type, type, expr, result);
- else
- warning_at (loc, OPT_Wsign_conversion,
- "signed conversion from %qT to %qT changes "
- "the value of %qE",
- expr_type, type, expr);
+ if (check_sign)
+ give_warning = UNSAFE_SIGN;
}
else
give_warning = UNSAFE_OTHER;
@@ -1420,7 +1398,7 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
is a constant, it's type is not used in text of generated warnings
(otherwise they could sound misleading). */
return unsafe_conversion_p (loc, type, TREE_REALPART (expr), result,
- produce_warns);
+ check_sign);
/* Conversion from complex constant with non-zero imaginary part. */
else
{
@@ -1428,21 +1406,11 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
Perform checks for both real and imaginary parts. */
if (TREE_CODE (type) == COMPLEX_TYPE)
{
- /* Unfortunately, produce_warns must be false in two subsequent
- calls of unsafe_conversion_p, because otherwise we could
- produce strange "double" warnings, if both real and imaginary
- parts have conversion problems related to signedness.
-
- For example:
- int32_t _Complex a = 0x80000000 + 0x80000000i;
-
- Possible solution: add a separate function for checking
- constants and combine result of two calls appropriately. */
enum conversion_safety re_safety =
- unsafe_conversion_p (loc, type, TREE_REALPART (expr),
- result, false);
+ unsafe_conversion_p (loc, type, TREE_REALPART (expr),
+ result, check_sign);
enum conversion_safety im_safety =
- unsafe_conversion_p (loc, type, imag_part, result, false);
+ unsafe_conversion_p (loc, type, imag_part, result, check_sign);
/* Merge the results into appropriate single warning. */
@@ -1524,16 +1492,16 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
/* When they are the same width but different signedness,
then the value may change. */
- else if (((TYPE_PRECISION (type) == TYPE_PRECISION (expr_type)
+ else if ((TYPE_PRECISION (type) == TYPE_PRECISION (expr_type)
&& 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)))
- && produce_warns)
- warning_at (loc, OPT_Wsign_conversion, "conversion to %qT from %qT "
- "may change the sign of the result",
- type, expr_type);
+ || (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (expr_type)))
+ {
+ if (check_sign)
+ give_warning = UNSAFE_SIGN;
+ }
}
/* Warn for integer types converted to real types if and only if
@@ -1588,14 +1556,13 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
/* Check for different signedness, see case for real-domain
integers (above) for a more detailed comment. */
- else if (((TYPE_PRECISION (to_type) == TYPE_PRECISION (from_type)
- && TYPE_UNSIGNED (to_type) != TYPE_UNSIGNED (from_type))
- || (TYPE_UNSIGNED (to_type) && !TYPE_UNSIGNED (from_type)))
- && produce_warns)
- warning_at (loc, OPT_Wsign_conversion,
- "conversion to %qT from %qT "
- "may change the sign of the result",
- type, expr_type);
+ else if ((TYPE_PRECISION (to_type) == TYPE_PRECISION (from_type)
+ && TYPE_UNSIGNED (to_type) != TYPE_UNSIGNED (from_type))
+ || (TYPE_UNSIGNED (to_type) && !TYPE_UNSIGNED (from_type)))
+ {
+ if (check_sign)
+ give_warning = UNSAFE_SIGN;
+ }
}
else if (TREE_CODE (from_type) == INTEGER_TYPE
&& TREE_CODE (to_type) == REAL_TYPE
diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index bd749579e95..6dbc660ddb4 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -1210,7 +1210,39 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
else
break;
- if (TREE_CODE_CLASS (TREE_CODE (result)) == tcc_constant)
+ if (conversion_kind == UNSAFE_SIGN)
+ {
+ bool cstresult
+ = (result
+ && TREE_CODE_CLASS (TREE_CODE (result)) == tcc_constant);
+ if (TYPE_UNSIGNED (type))
+ {
+ if (cstresult)
+ warning_at (loc, OPT_Wsign_conversion,
+ "unsigned conversion from %qT to %qT "
+ "changes value from %qE to %qE",
+ expr_type, type, expr, result);
+ else
+ warning_at (loc, OPT_Wsign_conversion,
+ "unsigned conversion from %qT to %qT "
+ "changes the value of %qE",
+ expr_type, type, expr);
+ }
+ else
+ {
+ if (cstresult)
+ warning_at (loc, OPT_Wsign_conversion,
+ "signed conversion from %qT to %qT changes "
+ "value from %qE to %qE",
+ expr_type, type, expr, result);
+ else
+ warning_at (loc, OPT_Wsign_conversion,
+ "signed conversion from %qT to %qT changes "
+ "the value of %qE",
+ expr_type, type, expr);
+ }
+ }
+ else if (TREE_CODE_CLASS (TREE_CODE (result)) == tcc_constant)
warning_at (loc, warnopt,
"conversion from %qT to %qT changes value from %qE to %qE",
expr_type, type, expr, result);
@@ -1234,23 +1266,29 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
default: /* 'expr' is not a constant. */
conversion_kind = unsafe_conversion_p (loc, type, expr, result, true);
- if (conversion_kind == UNSAFE_IMAGINARY)
- warning_at (loc, OPT_Wconversion,
- "conversion from %qT to %qT discards imaginary component",
- expr_type, type);
- else
- {
- int warnopt;
- if (conversion_kind == UNSAFE_REAL)
- warnopt = OPT_Wfloat_conversion;
- else if (conversion_kind)
- warnopt = OPT_Wconversion;
- else
- break;
+ {
+ int warnopt;
+ if (conversion_kind == UNSAFE_REAL)
+ warnopt = OPT_Wfloat_conversion;
+ else if (conversion_kind == UNSAFE_SIGN)
+ warnopt = OPT_Wsign_conversion;
+ else if (conversion_kind)
+ warnopt = OPT_Wconversion;
+ else
+ break;
+ if (conversion_kind == UNSAFE_SIGN)
+ warning_at (loc, warnopt, "conversion to %qT from %qT "
+ "may change the sign of the result",
+ type, expr_type);
+ else if (conversion_kind == UNSAFE_IMAGINARY)
+ warning_at (loc, warnopt,
+ "conversion from %qT to %qT discards imaginary component",
+ expr_type, type);
+ else
warning_at (loc, warnopt,
"conversion from %qT to %qT may change value",
expr_type, type);
- }
+ }
}
}
diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index ca6609ff4d2..7acf8a1abcb 100644
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,8 @@
+2020-01-10 Jason Merrill <ja...@redhat.com>
+
+ * c-common.c (unsafe_conversion_p): Don't warn, return UNSAFE_SIGN.
+ * c-warn.c (conversion_warning): Warn about UNSAFE_SIGN.
+
2020-01-01 Jakub Jelinek <ja...@redhat.com>
Update copyright years.
commit 3b4de797628e4d5b3e7068e614762e2a9a97c314
Author: Jason Merrill <ja...@redhat.com>
Date: Thu Jan 9 22:28:39 2020 -0500
Shorten right-shift again in C++.
Back in SVN r131862 richi removed this code to fix PR 34235, but didn't
remove the parallel code from the C front-end because the bug had previously
been fixed in r44080. This patch copies the code from C again.
* typeck.c (cp_build_binary_op): Restore short_shift code.
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 9f4f535f41a..7b653cebca0 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4452,6 +4452,10 @@ cp_build_binary_op (const op_location_t &location,
Also implies COMMON. */
int short_compare = 0;
+ /* Nonzero if this is a right-shift operation, which can be computed on the
+ original short and then promoted if the operand is a promoted short. */
+ int short_shift = 0;
+
/* Nonzero means set RESULT_TYPE to the common type of the args. */
int common = 0;
@@ -4844,6 +4848,9 @@ cp_build_binary_op (const op_location_t &location,
}
else
{
+ if (!integer_zerop (op1))
+ short_shift = 1;
+
if (compare_tree_int (const_op1, TYPE_PRECISION (type0)) >= 0
&& (complain & tf_warning)
&& c_inhibit_evaluation_warnings == 0)
@@ -5586,6 +5593,37 @@ cp_build_binary_op (const op_location_t &location,
shorten == -1);
}
+ /* Shifts can be shortened if shifting right. */
+
+ if (short_shift)
+ {
+ int unsigned_arg;
+ tree arg0 = get_narrower (op0, &unsigned_arg);
+
+ final_type = result_type;
+
+ if (arg0 == op0 && final_type == TREE_TYPE (op0))
+ unsigned_arg = TYPE_UNSIGNED (TREE_TYPE (op0));
+
+ if (TYPE_PRECISION (TREE_TYPE (arg0)) < TYPE_PRECISION (result_type)
+ && tree_int_cst_sgn (op1) > 0
+ /* We can shorten only if the shift count is less than the
+ number of bits in the smaller type size. */
+ && compare_tree_int (op1, TYPE_PRECISION (TREE_TYPE (arg0))) < 0
+ /* We cannot drop an unsigned shift after sign-extension. */
+ && (!TYPE_UNSIGNED (final_type) || unsigned_arg))
+ {
+ /* Do an unsigned shift if the operand was zero-extended. */
+ result_type
+ = c_common_signed_or_unsigned_type (unsigned_arg,
+ TREE_TYPE (arg0));
+ /* Convert value-to-be-shifted to that type. */
+ if (TREE_TYPE (op0) != result_type)
+ op0 = convert (result_type, op0);
+ converted = 1;
+ }
+ }
+
/* Comparison operations are shortened too but differently.
They identify themselves by setting short_compare = 1. */
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 210ab4e619f..2156d692652 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,7 @@
+2020-01-10 Jason Merrill <ja...@redhat.com>
+
+ * typeck.c (cp_build_binary_op): Restore short_shift code.
+
2020-01-08 Jason Merrill <ja...@redhat.com>
* cp-gimplify.c (cp_gimplify_expr) [TARGET_EXPR]: Check
