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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

@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

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 ^`.)

To the target audience of this warning, the news about `^`'s semantics would be coming as a surprise. Not every user of C++ is a power user; and only power users would be like "I know `10 ^ alpha` means `alpha xor 0xA` and in fact that's exactly what I meant — please don't warn me about my code!"

However, if this is the version that gets committed, it's far better than nothing, and it can always be further tightened up later. (For example, this version doesn't yet warn about `bigpower3` et seq., right? It basically //only// hits the literal cases.)


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

https://reviews.llvm.org/D66397