[PATCH] D97445: [clang][sema] Ignore xor-used-as-pow if both sides are macros

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: xbolva00, aaron.ballman.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I've read both https://reviews.llvm.org/D63423 and https://reviews.llvm.org/D66397, but I think both of them miss the case where both RHS and LHS of the XOR are macros, and thus the warning should not be emitted (I think).

I ran into this while compiling elfutils with clang and all the available changes to the code seem non-sensical and the warning in clang seems misplaced given that both operands are macros, even if the left side evaluates to 2 and the right side to 10, for example. They are just used as state.

I know this warning (in conjunction with macros especially) has been discussed a lot in the past (as far as I know regarding the Chrome code base), but this patch is very conservative regarding the change to the warning semantics. And the case it changes was not even covered by the tests. I've added a test for it now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97445

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/warn-xor-as-pow.cpp


Index: clang/test/SemaCXX/warn-xor-as-pow.cpp
===================================================================
--- clang/test/SemaCXX/warn-xor-as-pow.cpp
+++ clang/test/SemaCXX/warn-xor-as-pow.cpp
@@ -65,6 +65,7 @@
 
   res = 2 ^ 0x4;
   res = 2 ^ 04;
+  res = TWO ^ TEN;
   res = 0x2 ^ 10;
   res = 0X2 ^ 10;
   res = 02 ^ 10;
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -12102,6 +12102,11 @@
   if (Loc.isMacroID())
     return;
 
+  // Do not diagnose if both LHS and RHS are macros
+  if (XorLHS.get()->getExprLoc().isMacroID() &&
+      XorRHS.get()->getExprLoc().isMacroID())
+    return;
+
   bool Negative = false;
   bool ExplicitPlus = false;
   const auto *LHSInt = dyn_cast<IntegerLiteral>(XorLHS.get());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97445.326294.patch
Type: text/x-patch
Size: 846 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210225/d4ed4095/attachment-0001.bin>