Commit 731dbfc3 by Jason Merrill

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.
parent 0501b742
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.
2020-01-20 Nathan Sidwell <nathan@acm.org> 2020-01-20 Nathan Sidwell <nathan@acm.org>
PR preprocessor/80005 PR preprocessor/80005
......
...@@ -1312,9 +1312,9 @@ int_safely_convertible_to_real_p (const_tree from_type, const_tree to_type) ...@@ -1312,9 +1312,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 * EXPR is not a constant of integer type which cannot be
exactly converted to real type. exactly converted to real type.
Function allows conversions between types of different signedness and Function allows conversions between types of different signedness if
can return SAFE_CONVERSION (zero) in that case. Function can produce CHECK_SIGN is false and can return SAFE_CONVERSION (zero) in that
signedness warnings if PRODUCE_WARNS is true. case. Function can return UNSAFE_SIGN if CHECK_SIGN is true.
RESULT, when non-null is the result of the conversion. When constant RESULT, when non-null is the result of the conversion. When constant
it is included in the text of diagnostics. it is included in the text of diagnostics.
...@@ -1325,14 +1325,11 @@ int_safely_convertible_to_real_p (const_tree from_type, const_tree to_type) ...@@ -1325,14 +1325,11 @@ int_safely_convertible_to_real_p (const_tree from_type, const_tree to_type)
enum conversion_safety enum conversion_safety
unsafe_conversion_p (location_t loc, tree type, tree expr, tree result, 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 */ enum conversion_safety give_warning = SAFE_CONVERSION; /* is 0 or false */
tree expr_type = TREE_TYPE (expr); 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); loc = expansion_point_location_if_in_system_header (loc);
expr = fold_for_warn (expr); expr = fold_for_warn (expr);
...@@ -1360,32 +1357,13 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result, ...@@ -1360,32 +1357,13 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
if (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (expr_type) if (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (expr_type)
&& tree_int_cst_sgn (expr) < 0) && tree_int_cst_sgn (expr) < 0)
{ {
if (produce_warns) if (check_sign)
{ give_warning = UNSAFE_SIGN;
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 (!TYPE_UNSIGNED (type) && TYPE_UNSIGNED (expr_type)) else if (!TYPE_UNSIGNED (type) && TYPE_UNSIGNED (expr_type))
{ {
if (cstresult) if (check_sign)
warning_at (loc, OPT_Wsign_conversion, give_warning = UNSAFE_SIGN;
"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 else
give_warning = UNSAFE_OTHER; give_warning = UNSAFE_OTHER;
...@@ -1425,7 +1403,7 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result, ...@@ -1425,7 +1403,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 is a constant, it's type is not used in text of generated warnings
(otherwise they could sound misleading). */ (otherwise they could sound misleading). */
return unsafe_conversion_p (loc, type, TREE_REALPART (expr), result, return unsafe_conversion_p (loc, type, TREE_REALPART (expr), result,
produce_warns); check_sign);
/* Conversion from complex constant with non-zero imaginary part. */ /* Conversion from complex constant with non-zero imaginary part. */
else else
{ {
...@@ -1433,21 +1411,11 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result, ...@@ -1433,21 +1411,11 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
Perform checks for both real and imaginary parts. */ Perform checks for both real and imaginary parts. */
if (TREE_CODE (type) == COMPLEX_TYPE) 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 = enum conversion_safety re_safety =
unsafe_conversion_p (loc, type, TREE_REALPART (expr), unsafe_conversion_p (loc, type, TREE_REALPART (expr),
result, false); result, check_sign);
enum conversion_safety im_safety = 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. */ /* Merge the results into appropriate single warning. */
...@@ -1530,15 +1498,13 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result, ...@@ -1530,15 +1498,13 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
/* When they are the same width but different signedness, /* When they are the same width but different signedness,
then the value may change. */ 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)) && TYPE_UNSIGNED (expr_type) != TYPE_UNSIGNED (type))
/* Even when converted to a bigger type, if the type is /* Even when converted to a bigger type, if the type is
unsigned but expr is signed, then negative values unsigned but expr is signed, then negative values
will be changed. */ will be changed. */
|| (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (expr_type))) || (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (expr_type)))
&& produce_warns) && check_sign)
warning_at (loc, OPT_Wsign_conversion, "conversion to %qT from %qT " give_warning = UNSAFE_SIGN;
"may change the sign of the result",
type, expr_type);
} }
/* Warn for integer types converted to real types if and only if /* Warn for integer types converted to real types if and only if
...@@ -1594,13 +1560,10 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result, ...@@ -1594,13 +1560,10 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
/* Check for different signedness, see case for real-domain /* Check for different signedness, see case for real-domain
integers (above) for a more detailed comment. */ integers (above) for a more detailed comment. */
else if (((TYPE_PRECISION (to_type) == TYPE_PRECISION (from_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))
|| (TYPE_UNSIGNED (to_type) && !TYPE_UNSIGNED (from_type))) || (TYPE_UNSIGNED (to_type) && !TYPE_UNSIGNED (from_type)))
&& produce_warns) && check_sign)
warning_at (loc, OPT_Wsign_conversion, give_warning = UNSAFE_SIGN;
"conversion to %qT from %qT "
"may change the sign of the result",
type, expr_type);
} }
else if (TREE_CODE (from_type) == INTEGER_TYPE else if (TREE_CODE (from_type) == INTEGER_TYPE
&& TREE_CODE (to_type) == REAL_TYPE && TREE_CODE (to_type) == REAL_TYPE
......
...@@ -784,8 +784,7 @@ enum conversion_safety { ...@@ -784,8 +784,7 @@ enum conversion_safety {
SAFE_CONVERSION = 0, SAFE_CONVERSION = 0,
/* Another type of conversion with problems. */ /* Another type of conversion with problems. */
UNSAFE_OTHER, UNSAFE_OTHER,
/* Conversion between signed and unsigned integers /* Conversion between signed and unsigned integers. */
which are all warned about immediately, so this is unused. */
UNSAFE_SIGN, UNSAFE_SIGN,
/* Conversions that reduce the precision of reals including conversions /* Conversions that reduce the precision of reals including conversions
from reals to integers. */ from reals to integers. */
......
...@@ -1210,7 +1210,39 @@ conversion_warning (location_t loc, tree type, tree expr, tree result) ...@@ -1210,7 +1210,39 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
else else
break; 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, warning_at (loc, warnopt,
"conversion from %qT to %qT changes value from %qE to %qE", "conversion from %qT to %qT changes value from %qE to %qE",
expr_type, type, expr, result); expr_type, type, expr, result);
...@@ -1234,23 +1266,29 @@ conversion_warning (location_t loc, tree type, tree expr, tree result) ...@@ -1234,23 +1266,29 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
default: /* 'expr' is not a constant. */ default: /* 'expr' is not a constant. */
conversion_kind = unsafe_conversion_p (loc, type, expr, result, true); conversion_kind = unsafe_conversion_p (loc, type, expr, result, true);
if (conversion_kind == UNSAFE_IMAGINARY) {
warning_at (loc, OPT_Wconversion, int warnopt;
"conversion from %qT to %qT discards imaginary component", if (conversion_kind == UNSAFE_REAL)
expr_type, type); warnopt = OPT_Wfloat_conversion;
else else if (conversion_kind == UNSAFE_SIGN)
{ warnopt = OPT_Wsign_conversion;
int warnopt; else if (conversion_kind)
if (conversion_kind == UNSAFE_REAL) warnopt = OPT_Wconversion;
warnopt = OPT_Wfloat_conversion; else
else if (conversion_kind) break;
warnopt = OPT_Wconversion; if (conversion_kind == UNSAFE_SIGN)
else warning_at (loc, warnopt, "conversion to %qT from %qT "
break; "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, warning_at (loc, warnopt,
"conversion from %qT to %qT may change value", "conversion from %qT to %qT may change value",
expr_type, type); expr_type, type);
} }
} }
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment