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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to