[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

[Bruno Ricci via Phabricator via cfe-commits]

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