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

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

On 09/27/16 14:49, Florian Weimer wrote:
> * Jason Merrill:
>
>> On Sun, Sep 25, 2016 at 3:46 AM, Bernd Edlinger
>> <bernd.edlinger@hotmail.de> wrote:
>>> This patch makes -Wint-in-bool-context warn on suspicious integer left
>>> shifts, when the integer is signed, which is most likely some kind of
>>> programming error, for instance using "<<" instead of "<".
>>>
>>> The warning is motivated by the fact, that an overflow on integer shift
>>> left is undefined behavior, even if gcc won't optimize the shift based
>>> on the undefined behavior.
>>>
>>> So in absence of undefined behavior the boolean result does not depend
>>> on the shift value, thus the whole shifting is pointless.
>>
>> It's pointless for unsigned integers, too; why not warn for them as
>> well?  And why not warn for 0 << 0 and 1 << 0, which are just as
>> pointless?
>
> “1 << 0“ is often used in a sequence of flag mask definitions.  This
> example is from <bits/termios.h>:
>
> | /* Terminal control structure.  */
> | struct termios
> | {
> |   /* Input modes.  */
> |   tcflag_t c_iflag;
> | #define IGNBRK  (1 << 0)        /* Ignore break condition.  */
> | #define BRKINT  (1 << 1)        /* Signal interrupt on break.  */
> | #define IGNPAR  (1 << 2)        /* Ignore characters with parity errors.  */
> | #define PARMRK  (1 << 3)        /* Mark parity and framing errors.  */
>
> “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....



Bernd.