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

Arthur O'Dwyer via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 21 13:21:51 PDT 2019


Quuxplusone added a comment.

In D66397#1639840 <https://reviews.llvm.org/D66397#1639840>, @xbolva00 wrote:

> Swap trick is cool, we can suggest it always(vs. ‘xor’)


Well, you should avoid suggesting that the user rewrite `2 ^ 10` as `10 ^ 2`.
Anyway, I think some of the observers here might be confusing two aspects of the issue.

First there's "how can the programmer shut up this diagnostic?", to which there are dozens of answers that all work equally well. `0x2 ^ 3`, `2 ^ 0x3`, `2 xor 3`, `3 ^ 2` (in this case), `myxorfunction(2, 3)`, `02 ^ 03`, `0b10 ^ 0b11`, Richard's suggested de-cleverfication of `3 - 2`, and so on. Basically anything that convinces the compiler you're not trying to take an integer power of 2.

Second there's "what advice and/or fixit should Clang produce to push the programmer toward //one specific way// of shutting up this diagnostic?". Right now, IIUC, Clang always gives an English note suggesting to change the left-hand side into hex (preserving the behavior of the code), //and// a technical fixit suggesting to use `1 << RHS` or `1eRHS` (changing the behavior of the code).

The main trouble we're having with #2 is that when Clang suggests //one// possible fix, the Chromium programmers seemed to interpret that as meaning that that was the //only// possible fix — they didn't look deeper to see if there might be other possible ways to improve the code.

Perhaps you could change the English note to say

> to silence this warning, replace expression with '0x2 ^ ALPHA_OFFSET' **or use the 'xor' keyword**

This would at least give more of a jog to the programmer's brain, by saying "you have a choice to make here; don't apply the compiler's suggested fixit blindly."


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

https://reviews.llvm.org/D66397





More information about the cfe-commits mailing list