riccibruno added a comment.
> This should not warn. Please verify that patch was applied correctly and you
> use newly built clang with this patch. (I use arc patch DXXXXX)
You were right, I messed up on my side. Sorry about the noise !
I don't have much to add to the macro yes/no discussion but I have added some
inline comments.
================
Comment at: lib/Sema/SemaExpr.cpp:9449
+ }
+
+ if (LHSInt->getValue().getBitWidth() != RHSInt->getValue().getBitWidth())
----------------
This will miss literals with a unary +, like `10 ^ +8`. I am not finding any
code sample on `codesearch.isocpp.org` with this pattern, but you could imagine
someone writing code like :
```
#define EXPONENT 10
res1 = 10 ^ +EXPONENT;
res2 = 10 ^ -EXPONENT;
```
WDYT ?
================
Comment at: lib/Sema/SemaExpr.cpp:9469
+ const llvm::APInt XorValue = LeftSideValue ^ RightSideValue;
+
+ std::string LHSStr = Lexer::getSourceText(
----------------
You could bailout early if the values are such that no diagnostics is ever
going to be issued, before doing any source range/string manipulations (ie: do
all the tests on the values first).
================
Comment at: lib/Sema/SemaExpr.cpp:9506
+ return;
+
+ bool SuggestXor = S.getLangOpts().CPlusPlus ||
S.getPreprocessor().isMacroDefined("xor");
----------------
Maybe put the slightly more expensive `find` last in the condition.
================
Comment at: lib/Sema/SemaExpr.cpp:9674
+ RHS.get());
+
// Enforce type constraints: C99 6.5.6p3.
----------------
I think that this will miss code like `(2 ^ 64) - 1u` since `IgnoreParens()`
will not skip any implicit casts added by `UsualArithmeticConversions`. (Also
`IgnoreParens()` skips a bunch of other things, but it does not seem relevant
here).
Also, in `CheckBitwiseOperands` `diagnoseXorMisusedAsPow` is called before
`UsualArithmeticConversions` and not after. I am wondering if it is possible
for `diagnoseXorMisusedAsPow` to be called with invalid arguments in
`CheckBitwiseOperands` ?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66397/new/
https://reviews.llvm.org/D66397
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
