On 1 May 2012 16:00, Joseph S. Myers <[email protected]> wrote:
> On Tue, 1 May 2012, Manuel López-Ibáñez wrote:
>
>> > Are you sure you want STRIP_NOPS rather than STRIP_SIGN_NOPS here? If so,
>> > could you ensure there are comments explaining why removing sign changes
>> > is safe in this context?
>>
>> For getting the original enumeral type of a expr, why would sign changes
>> matter?
>
> What if the comparison is
>
> (unsigned) (expr_of_signed_enum_type) >= 0
>
> ? (With GCC, the enum will have a signed type if one of its values is
> negative.) That seems worth a warning - the point of the patch as I
> understand it is to avoid warning for
What I understood from the PR is that we should never warn for enums.
But if you think we should warn for the above, using STRIPS_SIGN_NOPS
does warn for this and avoids the warning in the original testcase. Is
this version OK?
+/* Given an expression as a tree, return its original type. Do this
+ by stripping any conversion that generates no instruction but don't
+ let the signedness change. */
+static tree
+expr_original_type (tree expr)
+{
+ STRIP_SIGN_NOPS (expr);
+ return TREE_TYPE (expr);
+}
+
(The important and hard to fix case is anyway to not warn for >= FOO,
which triggers more often and it is even more annoying and it is also
the one that Clang fixes and not this one.)
Cheers,
Manuel.
