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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
