[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[Harald van Dijk via Phabricator via llvm-commits]

hvdijk added a comment.

In D63423#2732158 <https://reviews.llvm.org/D63423#2732158>, @Quuxplusone wrote:

> In D63423#2732152 <https://reviews.llvm.org/D63423#2732152>, @hvdijk wrote:
>
>> It's bad enough that this warns for
>>
>> #define A 2
>> int f() { return A ^ 1; }
>>
>> where as far as the users of A are concerned...
>
> I see how this warning is arguably overzealous in the //very special case// of "raising" to the constant `1` (since nobody would ever write that by accident). However, if the user of A wants to indicate that they understand this is bitwise-xor, they can simply change the body of their `f` to `return A ^ 0x1;` — hex notation suppresses the warning.  (Changing the definition of `A` as well is perhaps a good idea but not technically required.)  IMO this is good enough and we should leave it.  (What do you think, having seen the `A ^ 0x1` workaround? Does that sufficiently address your needs?)

I missed that hex notation for the RHS also suppresses the warning. Thanks, that's good to know. Combined with the fact that the case where LHS and RHS are both macros now no longer triggers the warning, that means one of LHS or RHS must be an integer literal, so there will always be a viable workaround once clang 13 is released. That may be good enough.

>> [...] I'm not seeing from the previous discussion whether this case was based on real world programmer errors or not
>
> The description links to a couple of tweets showing examples from the wild:
> https://twitter.com/jfbastien/status/1139298419988549632
> https://twitter.com/mikemx7f/status/1139335901790625793

Those are different cases, they literally have `2 ^` in there. In the case I was asking about, the `2` comes from a macro expansion, but the `^` does not. That is much less likely to be a mistake, and one where I didn't see whether the warning for that was based on real-world concerns.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63423