[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

[Shivam Gupta via Phabricator via cfe-commits]

xgupta added a comment.

In D142609#4507696 <https://reviews.llvm.org/D142609#4507696>, @nathanchance wrote:

> I took the most recent version for a spin against the Linux kernel. The couple of issues that a previous revision of the warning flagged (https://github.com/ClangBuiltLinux/linux/issues/1806, https://github.com/ClangBuiltLinux/linux/issues/1807) are no longer visible (that seems intentional because both of those came from macro expressions) but I see a new one along a similar line as those:
>
> https://elixir.bootlin.com/linux/v6.5-rc2/source/drivers/gpu/drm/v3d/v3d_drv.h#L343
>
>   In file included from drivers/gpu/drm/v3d/v3d_bo.c:25:
>   drivers/gpu/drm/v3d/v3d_drv.h:343:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
>     343 |         if (NSEC_PER_SEC % HZ &&
>         |             ~~~~~~~~~~~~~~~~~ ^
>   drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: use '&' for a bitwise operation
>     343 |         if (NSEC_PER_SEC % HZ &&
>         |                               ^~
>         |                               &
>   drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: remove constant to silence this warning
>   1 warning generated.
>
> Another minimized example showing how the warning can trigger with different configurations:
>
>   $ cat x.c
>   #define A 1000
>   #define B 300
>   #define C 250
>   #define D 100
>   
>   int bar(void);
>   int baz(void);
>   
>   int foo(void)
>   {
>           if (A % B && bar()) // 1000 % 300 = 100, warning
>                   return -3;
>   
>           if (A % C && bar()) // 1000 % 250 = 0, no warning
>                   return -2;
>   
>           if (A % D && bar()) // 1000 % 100 = 0, no warning
>                   return -1;
>   
>           return baz();
>   }
>   
>   $ clang -Wall -fsyntax-only x.c
>   x.c:11:12: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
>      11 |         if (A % B && bar())
>         |             ~~~~~ ^
>   x.c:11:12: note: use '&' for a bitwise operation
>      11 |         if (A % B && bar())
>         |                   ^~
>         |                   &
>   x.c:11:12: note: remove constant to silence this warning
>   1 warning generated.
>
> I am sure we can send a patch making that a boolean expression (although it is duplicated in a few places it seems so multiple patches will be needed) but I figured I would report it just in case there was something that should be changed with the warning, since I see there was already some discussion around not warning for macros and this seems along a similar line.

WDYT @xbolva00,  It is a valid warning or more modification is required in the patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142609/new/

https://reviews.llvm.org/D142609