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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D66397#1639818 <https://reviews.llvm.org/D66397#1639818>, @rsmith wrote:

> The `ALPHA_OFFSET` code seems to be unnecessarily "clever" to me. I think the warning can be suppressed by reversing the operands:
>
> `ALPHA_OFFSET ^ 2`
>
> But if I were maintaining that code I would get rid of the xor hack entirely and use
>
>   #define CHANNEL_OFFSET(i) (i)
>
>
> or
>
>   #define CHANNEL_OFFSET(i) (3-(i))
>


+1

In D66397#1641121 <https://reviews.llvm.org/D66397#1641121>, @thakis wrote:

> In D66397#1640685 <https://reviews.llvm.org/D66397#1640685>, @xbolva00 wrote:
>
> > >> I think adding parens and casts are fairly well-understood to suppress warnings.
> >
> > It should work here as well. #define ALPHA_OFFSET (3). Did anobody from Chromium try it?
> >
> > >> They have varying levels of C++ proficiency
> >
> > I consider things like hex decimals or parens to silence as a quite basic knowledge.
>
>
> parens for sure, but I'm not aware of any other warning that behaves differently for ordinary and hex literals. What else is there?


I thought we did in Clang proper, but now I am second-guessing because I cannot find any with a quick search. Regardless, I think the "how do we silence this diagnostic" is getting a bit out of hand. We seem to now silence it when there's a hex, oct, or binary literal, a digit separator, or the `xor` token. I think we do not want to list all of those options in a diagnostic message.

> I looked through D63423 <https://reviews.llvm.org/D63423> and didn't find an evaluation of false / true positive rates when lhs and rhs are just literals vs when they're identifiers / macros. Glancing through https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=10+%5E&search=Search , I couldn't find a single true positive where lhs or rhs weren't a bare literal (but I didn't look super carefully). Did I just miss that in D63423 <https://reviews.llvm.org/D63423>? What was the motivation for firing on more than bare literals?

This is an interesting question to me that may help inform how we want to proceed.


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

https://reviews.llvm.org/D66397