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());
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());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
