On 08/14/2018 05:53 AM, Martin Jambor wrote:
Hi,
when you try compiling a call to function abs and provide an unsigned
int in the argument in C++, you will get an error about ambiguous
overload. In C however, it will pass without silently. The following
patch adds a warning for these cases, because I think it is likely that
such code does not do what the author intended.
I thought it a bit of an overkill to add a new warning group because of
this, so I added it to -Wtype-limits, which seemed the best fit. As a
consequence, the warning is on with -Wextra. The warning can be
suppressed with an explicit type-cast (which at the moment is implicit),
if someone really used it for some bit-tricks.
Bootstrapped and tested on x86_64-linux, also with make info. What do
you think, is this a good idea? Is it perhaps OK for trunk?
[Sorry, this turned into a longer response than I had planned
(and than I suspect you expected.]
The C++ errors are the result of the mess C++ has made of things
because of all the overloads it adds in different headers (I don't
say this to bash the language -- far from it). So I wouldn't look
to those as a model for warnings.
That said, I agree that calling abs() with an unsigned argument
could be a bug. Even calling abs() with a signed argument may
not be safe: abs(INT_MIN) is undefined, and passing in a wider
type will slice off the high end bits.
Detecting some of these bugs without too much noise would require
moving the warning out of the front-end and to some later point
after VRP has run.
+ else if (DECL_FUNCTION_CODE (expr.value) == BUILT_IN_ABS
+ && vec_safe_length (exprlist) == 1
+ && TYPE_UNSIGNED (TREE_TYPE ((*exprlist)[0])))
+ warning_at (expr_loc, OPT_Wtype_limits,
+ "calculation of an absolute value of "
+ "a value of unsigned type");
I would suggest to include more detail about the argument:
at a minimum, its type, and in the constant case (or in
the middle-end, when the range of the argument is known),
its value/range.
Following the example in overflow_warning() the warning in
the constant case might look something along the lines of:
warning_at (expr_loc, "conversion in an %qD expression "
"with argument %E of type %T" results in %E",
expr.value, arg0, TREE_TYPE (arg0), result);
and something like the following for arguments in a known
range (with VRP):
warning_at (expr_loc, "conversion in an %qD expression "
"with argument between %E and %E of type %T" "
"results in a negative value" between %E and %E,
expr.value, arg0_min, arg0_max, TREE_TYPE (arg0),
result_min, result_max);
(As an aside, the term "calculate" is more commonly used
colloquially and less frequently in reference to computers
or mathematics than "compute." In gcc.pot, there are just
6 occurrences of the former and 21 of the latter.)
Martin
