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

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

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.


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

https://reviews.llvm.org/D142609