On 1/22/20 1:20 PM, Jason Merrill wrote:
On 1/22/20 5:14 AM, Martin Liška wrote:
Hi.
Just for the record, after the change 526.blender_r fails due to:
blender/source/blender/blenlib/intern/math_color.c: In function
'rgb_float_to_uchar':
blender/source/blender/blenlib/BLI_utildefines.h:251:22: error:
conversion from 'float' to 'unsigned char' may change value
[-Werror=float-conversion]
251 | #define FTOCHAR(val) ((CHECK_TYPE_INLINE(val, float)), \
| ^
blender/source/blender/blenlib/BLI_utildefines.h:257:13: note: in
expansion of macro 'FTOCHAR'
257 | (v1)[0] =
FTOCHAR((v2[0])); \
| ^~~~~~~
blender/source/blender/blenlib/intern/math_color.c:421:2: note: in
expansion of macro 'F3TOCHAR3'
421 | F3TOCHAR3(col_f, r_col);
| ^~~~~~~~~
where the project sets -Werror=float-conversion dues to:
$ cat blender/source/blender/blenlib/BLI_strict_flags.h
#ifdef __GNUC__
# if (__GNUC__ * 100 + __GNUC_MINOR__) >= 406 /* gcc4.6+ only */
# pragma GCC diagnostic error "-Wsign-compare"
# pragma GCC diagnostic error "-Wconversion"
# endif
Thanks, investigating.
I wasn't able to reproduce this particular error, but I saw a different
one due to -funsigned-char, fixed thus:
commit 55b7df8bfb12938e7716445d4e2dc0d2ddf44bac
Author: Jason Merrill <ja...@redhat.com>
Date: Wed Jan 22 14:21:06 2020 -0500
c-family: Fix problems with blender and PPC from PR 40752 patch.
blender in SPEC is built with -funsigned-char, which is also the default on
PPC, and exposed -Wsign-conversion issues that weren't seen by the x86_64
testsuite. In blender we were complaining about operands to an expression
that we didn't't previously complain about as a whole. So only check
operands after we check the whole expression. Also, to fix the PR 40752
testcases on -funsigned-char targets, don't consider -Wsign-conversion for
the second operand of PLUS_EXPR, especially since fold changes
"x - 5" to "x + (-5)". And don't use SCHAR_MAX with plain char.
PR testsuite/93391 - PR 40752 test fails with unsigned plain char.
PR c++/40752
* c-warn.c (conversion_warning): Check operands only after checking
the whole expression. Don't check second operand of + for sign.
diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index 4df4893ca02..6c73317b4a6 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -1163,7 +1163,7 @@ 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;
+ int arith_ops = 0;
if (!warn_conversion && !warn_sign_conversion && !warn_float_conversion)
return false;
@@ -1266,14 +1266,8 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
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;
- }
+ arith_ops = 2;
+ goto default_;
case PREDECREMENT_EXPR:
case PREINCREMENT_EXPR:
@@ -1285,13 +1279,8 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
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;
- }
+ arith_ops = 1;
+ goto default_;
case COND_EXPR:
{
@@ -1304,11 +1293,7 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
|| conversion_warning (loc, type, op2, result));
}
- arith_op:
- /* We didn't warn about the operands, we might still want to warn if
- -Warith-conversion. */
- is_arith = true;
- gcc_fallthrough ();
+ default_:
default:
conversion_kind = unsafe_conversion_p (type, expr, result, true);
{
@@ -1321,11 +1306,27 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
warnopt = OPT_Wconversion;
else
break;
- if (is_arith
+
+ if (arith_ops
&& global_dc->option_enabled (warnopt,
global_dc->lang_mask,
global_dc->option_state))
- warnopt = OPT_Warith_conversion;
+ {
+ for (int i = 0; i < arith_ops; ++i)
+ {
+ tree op = TREE_OPERAND (expr, i);
+ tree opr = convert (type, op);
+ /* Avoid -Wsign-conversion for (unsigned)(x + (-1)). */
+ bool minus = TREE_CODE (expr) == PLUS_EXPR && i == 1;
+ if (unsafe_conversion_p (type, op, opr, !minus))
+ goto op_unsafe;
+ }
+ /* The operands seem safe, we might still want to warn if
+ -Warith-conversion. */
+ warnopt = OPT_Warith_conversion;
+ op_unsafe:;
+ }
+
if (conversion_kind == UNSAFE_SIGN)
warning_at (loc, warnopt, "conversion to %qT from %qT "
"may change the sign of the result",
diff --git a/gcc/testsuite/c-c++-common/Wconversion-pr40752.c b/gcc/testsuite/c-c++-common/Wconversion-pr40752.c
index dc757185c75..0cd1c7ba116 100644
--- a/gcc/testsuite/c-c++-common/Wconversion-pr40752.c
+++ b/gcc/testsuite/c-c++-common/Wconversion-pr40752.c
@@ -31,15 +31,15 @@ void bar(char c, int c2)
c >>= (int)1;
c <<= (int)1;
c <<= c2;
- c += ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */
+ c += ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */
c += c2; /* { dg-warning "conversion" } */
- c -= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */
+ c -= ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */
c -= c2; /* { dg-warning "conversion" } */
- c *= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */
+ c *= ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */
c *= c2; /* { dg-warning "conversion" } */
- c /= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */
+ c /= ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */
c /= c2; /* { dg-warning "conversion" } */
- c %= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */
+ c %= ((int)CHAR_MAX + CHAR_MAX); /* { 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
index 7b5c9dee617..cd70e34c390 100644
--- a/gcc/testsuite/c-c++-common/Wconversion-pr40752a.c
+++ b/gcc/testsuite/c-c++-common/Wconversion-pr40752a.c
@@ -31,15 +31,15 @@ void bar(char c, int c2)
c >>= (int)1;
c <<= (int)1; /* { dg-warning "conversion" } */
c <<= c2; /* { dg-warning "conversion" } */
- c += ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */
+ c += ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */
c += c2; /* { dg-warning "conversion" } */
- c -= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */
+ c -= ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */
c -= c2; /* { dg-warning "conversion" } */
- c *= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */
+ c *= ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */
c *= c2; /* { dg-warning "conversion" } */
- c /= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */
+ c /= ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */
c /= c2; /* { dg-warning "conversion" } */
- c %= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */
+ c %= ((int)CHAR_MAX + CHAR_MAX); /* { 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-pr40752b.c b/gcc/testsuite/c-c++-common/Wconversion-pr40752b.c
new file mode 100644
index 00000000000..208710bc772
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wconversion-pr40752b.c
@@ -0,0 +1,8 @@
+// PR c++/40752
+// { dg-additional-options -funsigned-char }
+
+#pragma GCC diagnostic error "-Wsign-conversion"
+void f(char *ar, int i)
+{
+ ar[i] -= 'a' - 'A';
+}
commit d9637168812939d6c9df29ce747d8d4648b37cef
Author: Jason Merrill <ja...@redhat.com>
Date: Wed Jan 22 14:12:55 2020 -0500
c-family: Remove location parm from unsafe_conversion_p.
My earlier change removed the warning calls from this function, so the
location is no longer useful.
* c-common.c (unsafe_conversion_p): Remove location parm.
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index fb0d53b079e..3e26ca034ca 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -853,8 +853,7 @@ extern tree c_common_signed_type (tree);
extern tree c_common_signed_or_unsigned_type (int, tree);
extern void c_common_init_ts (void);
extern tree c_build_bitfield_integer_type (unsigned HOST_WIDE_INT, int);
-extern enum conversion_safety unsafe_conversion_p (location_t, tree, tree, tree,
- bool);
+extern enum conversion_safety unsafe_conversion_p (tree, tree, tree, bool);
extern bool decl_with_nonnull_addr_p (const_tree);
extern tree c_fully_fold (tree, bool, bool *, bool = false);
extern tree c_wrap_maybe_const (tree, bool);
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 4a7f3a52335..774e29be104 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -1324,14 +1324,11 @@ int_safely_convertible_to_real_p (const_tree from_type, const_tree to_type)
to TYPE. */
enum conversion_safety
-unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
- bool check_sign)
+unsafe_conversion_p (tree type, tree expr, tree result, bool check_sign)
{
enum conversion_safety give_warning = SAFE_CONVERSION; /* is 0 or false */
tree expr_type = TREE_TYPE (expr);
- loc = expansion_point_location_if_in_system_header (loc);
-
expr = fold_for_warn (expr);
if (TREE_CODE (expr) == REAL_CST || TREE_CODE (expr) == INTEGER_CST)
@@ -1402,7 +1399,7 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
with different type of EXPR, but it is still safe, because when EXPR
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,
+ return unsafe_conversion_p (type, TREE_REALPART (expr), result,
check_sign);
/* Conversion from complex constant with non-zero imaginary part. */
else
@@ -1412,10 +1409,10 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
if (TREE_CODE (type) == COMPLEX_TYPE)
{
enum conversion_safety re_safety =
- unsafe_conversion_p (loc, type, TREE_REALPART (expr),
+ unsafe_conversion_p (type, TREE_REALPART (expr),
result, check_sign);
enum conversion_safety im_safety =
- unsafe_conversion_p (loc, type, imag_part, result, check_sign);
+ unsafe_conversion_p (type, imag_part, result, check_sign);
/* Merge the results into appropriate single warning. */
@@ -8068,7 +8065,7 @@ scalar_to_vector (location_t loc, enum tree_code code, tree op0, tree op1,
if (TREE_CODE (type0) == INTEGER_TYPE
&& TREE_CODE (TREE_TYPE (type1)) == INTEGER_TYPE)
{
- if (unsafe_conversion_p (loc, TREE_TYPE (type1), op0,
+ if (unsafe_conversion_p (TREE_TYPE (type1), op0,
NULL_TREE, false))
{
if (complain)
@@ -8117,7 +8114,7 @@ scalar_to_vector (location_t loc, enum tree_code code, tree op0, tree op1,
if (TREE_CODE (type0) == INTEGER_TYPE
&& TREE_CODE (TREE_TYPE (type1)) == INTEGER_TYPE)
{
- if (unsafe_conversion_p (loc, TREE_TYPE (type1), op0,
+ if (unsafe_conversion_p (TREE_TYPE (type1), op0,
NULL_TREE, false))
{
if (complain)
@@ -8133,7 +8130,7 @@ scalar_to_vector (location_t loc, enum tree_code code, tree op0, tree op1,
|| TREE_CODE (type0) == INTEGER_TYPE)
&& SCALAR_FLOAT_TYPE_P (TREE_TYPE (type1)))
{
- if (unsafe_conversion_p (loc, TREE_TYPE (type1), op0,
+ if (unsafe_conversion_p (TREE_TYPE (type1), op0,
NULL_TREE, false))
{
if (complain)
diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index d8f0ad654fe..4df4893ca02 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -1202,7 +1202,7 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
case INTEGER_CST:
case COMPLEX_CST:
{
- conversion_kind = unsafe_conversion_p (loc, type, expr, result, true);
+ conversion_kind = unsafe_conversion_p (type, expr, result, true);
int warnopt;
if (conversion_kind == UNSAFE_REAL)
warnopt = OPT_Wfloat_conversion;
@@ -1310,7 +1310,7 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
is_arith = true;
gcc_fallthrough ();
default:
- conversion_kind = unsafe_conversion_p (loc, type, expr, result, true);
+ conversion_kind = unsafe_conversion_p (type, expr, result, true);
{
int warnopt;
if (conversion_kind == UNSAFE_REAL)
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index aacd961faa1..009cb85ad60 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -5170,14 +5170,14 @@ build_conditional_expr_1 (const op_location_t &loc,
but the warnings (like Wsign-conversion) have already been
given by the scalar build_conditional_expr_1. We still check
unsafe_conversion_p to forbid truncating long long -> float. */
- if (unsafe_conversion_p (loc, stype, arg2, NULL_TREE, false))
+ if (unsafe_conversion_p (stype, arg2, NULL_TREE, false))
{
if (complain & tf_error)
error_at (loc, "conversion of scalar %qH to vector %qI "
"involves truncation", arg2_type, vtype);
return error_mark_node;
}
- if (unsafe_conversion_p (loc, stype, arg3, NULL_TREE, false))
+ if (unsafe_conversion_p (stype, arg3, NULL_TREE, false))
{
if (complain & tf_error)
error_at (loc, "conversion of scalar %qH to vector %qI "
