[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.


================
Comment at: lib/Sema/SemaExpr.cpp:10929
+    // Do not diagnose if xor keyword is used.
+    if (ExprStr.find("xor") != llvm::StringRef::npos)
+      return;
----------------
aaron.ballman wrote:
> xbolva00 wrote:
> > xbolva00 wrote:
> > > jfb wrote:
> > > > xbolva00 wrote:
> > > > > Quuxplusone wrote:
> > > > > > xbolva00 wrote:
> > > > > > > jfb wrote:
> > > > > > > > Doesn't this match any expression that contains `xor`? Put another way, I don't see `"xor"` used anywhere else under clang, what's usually done?
> > > > > > > Yes, but since xor is keyword in C++, I think this is safe.
> > > > > > I believe JF is worried about expressions like `constexpr int flexor_exponent = 3; return 10 ^ flexor_exponent;`. That expression contains the substring `"xor"` but does not use the `xor` keyword. Which reminds me, can you add some test cases showing what behavior you expect for operands that are not integer literals but still compile-time `const`, or `constexpr`, or template arguments?
> > > > > Ah, I will change it to “ xor “.
> > > > Spaces aren't the only valid whitespace character :)
> > > > 
> > > > ```
> > > > 2 xor
> > > >    31
> > > > ```
> > > I will rework it to chech only XOR op, not the while expr.
> > Well, 10 ^ flexor .. 
> > 
> >  We cannot reach xor check, flexor is not a integer literal. So I will rework nothing here.
> ```
> #define flexor 7
> ```
> Now `flexor` is an integer literal. (A test case for this would be beneficial.)
Also, as I've said above:
```
constexpr long long operator"" _xor(unsigned long long v)
{ return v; }
auto i = 10 ^ 5_xor;
```



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

https://reviews.llvm.org/D63423