Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops

[Jason Merrill <jason@redhat.com>]

On Wed, Sep 28, 2016 at 12:09 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> On 09/28/16 16:41, Jason Merrill wrote:
>> On Tue, Sep 27, 2016 at 11:10 AM, Bernd Edlinger
>> <bernd.edlinger@hotmail.de> wrote:
>>> On 09/27/16 16:42, Jason Merrill wrote:
>>>> On Tue, Sep 27, 2016 at 10:28 AM, Bernd Edlinger
>>>> <bernd.edlinger@hotmail.de> wrote:
>>>>> On 09/27/16 16:10, Florian Weimer wrote:
>>>>>> * Bernd Edlinger:
>>>>>>
>>>>>>>> “0 << 0” is used in a similar context, to create a zero constant for a
>>>>>>>> multi-bit subfield of an integer.
>>>>>>>>
>>>>>>>> This example comes from GDB, in bfd/elf64-alpha.c:
>>>>>>>>
>>>>>>>> |   insn = INSN_ADDQ | (16 << 21) | (0 << 16) | (0 << 0);
>>>>>>>>
>>>>>>>
>>>>>>> Of course that is not a boolean context, and will not get a warning.
>>>>>>>
>>>>>>> Question is if "if (1 << 0)" is possibly a miss-spelled "if (1 < 0)".
>>>>>>>
>>>>>>> Maybe 1 and 0 come from macro expansion....
>>>>>>
>>>>>> But what's the intent of treating 1 << 0 and 0 << 0 differently in the
>>>>>> patch, then?
>>>>>
>>>>> I am not sure if it was a good idea.
>>>>>
>>>>> I saw, we had code of the form
>>>>> bool flag = 1 << 2;
>>>>>
>>>>> another value LOOKUP_PROTECT is  1 << 0, and
>>>>> bool flag = 1 << 0;
>>>>>
>>>>> would at least not overflow the allowed value range of a boolean.
>>>>
>>>> Assigning a bit mask to a bool variable is still probably not what was
>>>> intended, even if it doesn't change the value.
>>>
>>> That works for me too.
>>> I can simply remove that exception.
>>
>> Sounds good.
>
> Great.  Is that an "OK with that change"?

What do you think about dropping the TYPE_UNSIGNED exception as well?
I don't see what difference that makes.

Jason