On 12/01/2016 12:51 AM, Jakub Jelinek wrote:
On Wed, Nov 30, 2016 at 06:14:14PM -0700, Martin Sebor wrote:
On 11/30/2016 12:01 PM, Jakub Jelinek wrote:
Hi!
This patch fixes some minor nits I've raised in the PR, more severe issues
left unresolved there.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Thank you. One comment below.
@@ -1059,7 +1048,12 @@ format_integer (const conversion_spec &s
}
if (code == NOP_EXPR)
- argtype = TREE_TYPE (gimple_assign_rhs1 (def));
+ {
+ tree type = TREE_TYPE (gimple_assign_rhs1 (def));
+ if (TREE_CODE (type) == INTEGER_TYPE
+ || TREE_CODE (type) == POINTER_TYPE)
+ argtype = type;
As I replied in my comment #6 on the bug, I'm not sure I see what
is wrong with the original code, and I haven't been able to come
up with a test case that demonstrates a problem because of it with
any of the types you mentioned (bool, enum, or floating).
I think for floating we don't emit NOP_EXPR, but FIX_TRUNC_EXPR;
Correct. The way to think about this stuff is whether or not the type
change is *likely* to lead to code generated. If the type change is not
likely to generate code, then NOP_EXPR is appropriate. Else a suitable
conversion is necessary. This distinction has long been something we'd
like to fix, but haven't gotten around it it.
It is almost always the case that a floating point conversion will
generate code. So NOP_EXPR is not appropriate for a floating point
conversion.
A NOP_EXPR will be used for a large number of integer conversions though.
perhaps bool/enum is fine, but in the UB case where one loads arbitrary
values into bool or enum the precision might be too small for those
(for enums only with -fstrict-enums).
Note that loading an out-of-range value into an enum object is, sadly,
not UB as long as there's enough storage space in the object to hold the
value. That's why enums often don't constrain ranges.
Jeff
