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

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D66397#1642012 <https://reviews.llvm.org/D66397#1642012>, @Quuxplusone wrote:

> @thakis wrote:
>
> > What was the motivation for firing on more than bare literals?
>
> Well, fundamentally, there is no difference in code quality between any of these snippets:
>
>   #define BIG 300
>   double bigpower1() { return 10 ^ BIG; }
>   
>   static constexpr int BIG = 300; double bigpower2() { return 10 ^ BIG; }
>   
>   double bigpower3(int BIG) { return 10 ^ BIG; }
>   
>   double bigpower4(int BIG) { return 10 ^ (BIG+1); }
>   
>   minval -= 10 ^ -precision;  // https://codesearch.isocpp.org/actcd19/main/q/qgis/qgis_2.18.28+dfsg-2/src/gui/editorwidgets/qgsrangewidgetwrapper.cpp
>   intermediate = (str[offset] - '0') / (10 ^ lpc);  // https://codesearch.isocpp.org/actcd19/main/p/pacemaker/pacemaker_1.1.18-2/lib/common/iso8601.c
>   real_loop += (((unsigned int) *argv[4]+k) - 48) * 10^(strlen(argv[4]) - (k+1));  // https://codesearch.isocpp.org/actcd19/main/liba/libaria/libaria_2.8.0+repack-1.2/tests/testCOM.cpp


Thanks for the real-world examples, these are convincing.

> If you seriously mean "xor", then you shouldn't be writing code that looks like you meant "exponentiate." I don't think it matters whether the LHS or RHS come from literals, macros, constexpr variables, const variables, function calls, additions, subtractions, or what-have-you. It seems reasonable, when you write `10 ^ x` or `2 ^ x` in C++, for the compiler to tell you about it so you can go fix it. (Admittedly it looks like a slippery slope to `int square(int n) { return n ^ 2; }` — but we can be data-driven here. There are no true-positive instances of `^ 2` in our corpus, whereas there are many true positives of `10 ^` and `2 ^`.)

Right, I agree with the "data-driven" bit. Hence my point that if in practice all true positives are from literals, then we should only warn on literals. The examples you posted show that that's not the case.

I now agree that it makes sense to warn when the operands are macros or variables.


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

https://reviews.llvm.org/D66397