Re: [PATCH] Make is_signed_type() simpler

From: Rasmus Villemoes
Date: Mon Sep 30 2019 - 02:16:59 EST


On 29/09/2019 23.09, Alexey Dobriyan wrote:
> On Sun, Sep 29, 2019 at 10:21:48PM +0200, Rasmus Villemoes wrote:
>> On 29/09/2019 22.06, Alexey Dobriyan wrote:
>>> * Simply compare -1 with 0,
>>> * Drop unnecessary parenthesis sets
>>>
>>> -#define is_signed_type(type) (((type)(-1)) < (type)1)
>>> +#define is_signed_type(type) ((type)-1 < 0)
>>
>> NAK. I wrote it that way to avoid -Wtautological-compare when type is
>> unsigned.
>
> Was is W=1?
>
> godbolt doesn't show it with just -Wall
>
> https://godbolt.org/z/kCA7mm
>
> And the warning which found i915 case is -Wextra not -Wtautological-compare.
>

Yeah, it's usually disabled/not enabled in the kernel. I did most of the
prototyping/testing in userspace with my default Cflags, and decided to
keep it this way in case somebody copy-pastes it to a code base that
does enable -Wtautological-compare. I see it's been copy-pasted to
tools/, who knows what they do. IMO, "it may be copy-pasted" would not
be valid reason for a transform the other way, but I really don't see a
reason for changing it now. Especially since it seems to require some
tree-wide adaptation.

Rasmus