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

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D63423#1550705 <https://reviews.llvm.org/D63423#1550705>, @aaron.ballman wrote:

> In D63423#1550697 <https://reviews.llvm.org/D63423#1550697>, @jfb wrote:
>
> > What I meant with macros was that I don't think we should warn on:
> >
> >   #define LEGIT(a, b) ({ work work work; a ^ b; work work work; })
> >  
> >   LEGIT(10, 5);
> >
> >
> > If the constants are inline in the macros then sure, warn there. Basically: if you literally wrote `CONSTANT ^ CONSTANT` and it looks fishy, let's warn. If token-pasting wrote it for you and it looks fishy, let's not warn.
>
>
> So you would not want to see this code diagnosed?
>
>   #define POW(x, y) (x ^ y)
>  
>   void f() {
>     int two_to_the_derp = POW(2, 16);
>   }
>
>
> I'm not certain I understand the rationale for why we would not want to diagnose such a construct. Do we have reason to expect a lot of false positives?


If you wrote `CONSTANT_2_OR_10 ^ CONSTANT` I'm pretty sure you messed up. Macros are opaque, and I'm not sure you clearly messed up anymore. Let's take the clear win, see what falls out, and only expand our reach if we have reason to do so. Without trying on huge codebases I don't know whether we'll get a lot of false positive. I'd rather be conservative and not cause headache.


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

https://reviews.llvm.org/D63423