Quuxplusone added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.cpp:41-42
+ Finder->addMatcher(
+ binaryOperation(hasAnyOperatorName("&&", "||", "!", "&", "|", "~", "^"))
+ .bind("operator"),
+ this);
----------------
`~` and `!` are not binary operations, right?
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/readability-use-alternative-tokens.rst:6-7
+
+Finds uses of symbol-based logical and bitwise operators and recommends using
+alternative tokens instead.
+
----------------
I strongly recommend splitting this check in two parts:
- Use `and`, `or`, `not` for logical operations.
- Use `bitand`, `bitor`, `xor`, `compl` for non-logical (bitwise) operations.
The reason I recommend this is that I have //seen// people (never in a real
codebase, mind you, but at least bloggers and such) recommend `and`/`or`/`not`
for logical operations; this is a long-standing convention in other languages
such as Python and Perl. Whereas I have //never// seen anyone recommend e.g.
`(x bitand compl mask)` over `(x & ~mask)`. So coupling the two ideas together
seems counterproductive, because //even if// someone wanted to use the Python
style in their codebase, they wouldn't be able to enforce it using this check,
because this check would be full of false positives related to the bitwise
operators.
The current check's recommendation to replace `(a || b) => (a or b)`, `(a ^ b)
=> (a xor b)` is particularly pernicious to non-language-lawyers, who might
assume that `or` and `xor` represented the same "part of speech" — either both
bitwise or both logical. I think this is a major motivation for the Pythonic
style: use English for logical `and or not`, and use symbols for mathematical
`& | ^ ~ << >> &= |= ~= <<= >>=`.
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/readability-use-alternative-tokens.rst:49-52
+Program composition
+-------------------
+
+This check doesn't yet account for program composition. This means that the
----------------
"Program composition" is not a term of art (except in the extremely general
sense of composing programs, i.e. programming). I recommend retitling this
section "Use of | as a pipe" or "Use of | with C++20 Ranges".
It might be worth adding a brief mention that the same is true of any
minigame/DSL embedded within C++; for example, this clang-tidy check will also
recommend changes like
```
qi::rule<std::string::iterator, boost::variant<int, bool>(),
ascii::space_type> value = qi::int_ | qi::bool_;
^
warning: use 'bitor' for bitwise disjunctions
```
and
```
std::fstream file("hello.txt", std::ios::in | std::ios::out);
^
warning: use 'bitor' for bitwise disjunctions
```
which are probably not desirable for the user-programmer.
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/readability-use-alternative-tokens.rst:58
+
+ // warning: use 'bitor' for logical disjunctions
+ auto evens = std::views::iota(0, 1'000)
----------------
s/logical/bitwise/
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107294/new/
https://reviews.llvm.org/D107294
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
