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

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 marked an inline comment as done.
xbolva00 added inline comments.


================
Comment at: lib/Sema/SemaExpr.cpp:11067
+
+  // Do not diagnose 2 ^ 64, but allow special case (2 ^ 64) - 1.
+  if (SubLHS && SubRHS && (LeftSideValue != 2 || RightSideValue != 64))
----------------
xbolva00 wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > This comment explains what the code does, but not why it does it. Given that we're adding special cases, I think more comments here explaining why this is valuable would be appreciated.
> > Thank you, the comments helped! But they also raised another question for me. What's special about 2^64? Why not (2^16) - 1 or other common power-of-two values? I would have expected 8, 16, 32, and 64 to be handled similarly.
> We generally suggest 1LL << C. But here we cant say 1LL << 64.
> 
> (2^16)-1 is diagnosed normally since we go here from “visit xor” code.
>  ^^^^
> 
> This (2^64)-1 handling was suggested by jfb.
> 
> I see no motivation cases to diagnose 2^65, 2^100, ...
> 
> 
> 
> 
I think the suggestion for (2^32)-1:
(1LL<32)-1 is good, or should we make things more complicated and suggest Int max macro? 


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

https://reviews.llvm.org/D66397