[PATCH] D107292: [clang] adds warning to alert user when they use alternative tokens as references

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D107292#2922599 <https://reviews.llvm.org/D107292#2922599>, @aaron.ballman wrote:

> In D107292#2922575 <https://reviews.llvm.org/D107292#2922575>, @dblaikie wrote:
>
>> In D107292#2921901 <https://reviews.llvm.org/D107292#2921901>, @aaron.ballman wrote:
>>
>>> In D107292#2920774 <https://reviews.llvm.org/D107292#2920774>, @dblaikie wrote:
>>>
>>>> In D107292#2920746 <https://reviews.llvm.org/D107292#2920746>, @cjdb wrote:
>>>>
>>>>> In D107292#2920637 <https://reviews.llvm.org/D107292#2920637>, @dblaikie wrote:
>>>>>
>>>>>> Not a huge objection - but minor quandry: What's the motivation for this patch? I don't know of any codebase that encourages/uses the alternative tokens and I wonder if adding more usability to them is a worthwhile investment in clang's codebase complexity, etc.
>>>>>
>>>>> There are codebases that use them (all of my non-Google, non-LLVM code does, for example, and I'm not the sole user: just a loud one who's also in a position to patch tooling).
>>>>
>>>> Ah, any pointers to large open source projects that use this?
>>>
>>> https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=bitand&search=Search
>>>
>>> (Searching for 'and' is a bit less useful because of how much it shows up in assembly, comments, etc.)
>>
>> Ah, cool. Only case I could find there (that wasn't C code or compiler test cases) was something called FuzzyLite (which looks like it hasn't been touched in 4 years or so).
>
> I suspect that `bitand` is used less than `and` because I believe bitwise operations are less common than logical ones.
>
>> I don't fundamentally object to this now it's being proposed as a clang-tidy thing - bar should be low/easy for experiments, getting user experience, adoption, etc.
>
> FWIW, my feeling is that *this* proposed patch is reasonable for Clang but the proposed patch for suggesting use of alternative tokens (or not) belongs in clang-tidy. This patch proposes a diagnostic that catches bugs and can be on by default without false positives, the other patch proposed diagnostics that were more about coding style.

Oh, sorry, got the two patches jumbled up. Right right.

Any data on a large codebase about whether this finds enough bugs* to be interesting? But yeah, I don't mind it terribly - seems low cost enough that I won't stand in the way of it if you're in favor.

- for some value of bug - that's been a point of debate for a while, turning on warnings like parentheses by default is an example. If we have this on by default, for codebases where it fires at all, are we creating busywork for people to go and cleanup code that's weird, but wasn't actively harmful/likely to be causing real problems for them? If we are, then that feels like it's more a stylistic thing than a bug-finding warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107292