[clang-tidy] adds warning to suggest users replace symbols with words

[PiotrZSL]

Summary

C++'s alternative tokens are a good way to improve readability and
minimise bugs by expressing intention using words instead of symbols
(which can both be more easily misspelt _and_ misread).

This warning analyses the binary logical and bitwise operators and warns
if they aren't spelt using the alternative tokens. Bitwise compound
assignment and != operators are not in scope since there's nothing to
misspel or misread.

Future work:

• add a way for the compiler to detect when the pipe operator (|) is not being used for a bitwise-or operation
• add preprocessor support
• add config options

[aaron.ballman]

The usual community viewpoint on off-by-default diagnostics is that they're generally low value and not something that users will manually enable. Because I don't think this will ever be reasonable to enable by default due to it inherently being a matter of style, I think this functionality is better handled by clang-tidy as a check in the readability module. I would note that it should also be supported in C (C99 and later) by also inserting #include <iso646.h> using the usual fix-it utilities.

[cjdb]

In D107294#2920355, @aaron.ballman wrote:

The usual community viewpoint on off-by-default diagnostics is that they're generally low value and not something that users will manually enable.

I was under the impression many warnings weren't enabled by default?

I think this functionality is better handled by clang-tidy

Ack. I've not touched clang-tidy yet; how different do you expect the code to look to what I've got right now?

[aaron.ballman]

In D107294#2920477, @cjdb wrote:

In D107294#2920355, @aaron.ballman wrote:

The usual community viewpoint on off-by-default diagnostics is that they're generally low value and not something that users will manually enable.

I was under the impression many warnings weren't enabled by default?

We certainly have some! My understanding is that many of those were added before we realized most people don't actually enable off-by-default warnings, so it's only "recently" (within the past 5-10 years) that we've pushed back more consistently on off-by-default diagnostics (and even then, we still allow them sometimes; -Wfallthrough is a good example of that).

I think this functionality is better handled by clang-tidy

Ack. I've not touched clang-tidy yet; how different do you expect the code to look to what I've got right now?

Because you're doing this at the AST node level, I would expect it to be pretty easy. If you want to do it within preprocessor expressions (or other places where the information isn't in the AST node), it's still doable but might be a bit more complex.

[cjdb]

moves patch from Clang to clang-tidy

[whisperity]

clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.h

5–6

This seems to be very old code, there was a licence change approx. 2 years ago.

20–22

But also there is a convention for the documentation comment for the main class of checks, and the code here should adhere to that.

clang-tools-extra/docs/clang-tidy/checks/readability-use-alternative-tokens.rst

14

What does this refer to? Perhaps a code example and a clearly indicated Limitations heading would be more clear to indicate whether someone would want to enable this check.

21–27

Either use plural xor singular for all of these printouts, to keep consistency.

[cjdb]

clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.h

5–6

Yikes, that's what I get for not reading what I'm copying. Should there be an NFC to go and update the other checks' licences too?

20–22

Not sure I'm following you here: are you suggesting I put the contents of my rst file in a comment here?

clang-tools-extra/docs/clang-tidy/checks/readability-use-alternative-tokens.rst

21–27

negation is the plural in this case.

[cjdb]

• updates licences
• updates documentation

[cjdb]

removes obsolete code

[aaron.ballman]

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

133–134

I think this might be a case where we want the check to either recommend using alternative tokens or recommend against using alternative tokens via a configuration option (so users can control readability in either direction). If you agree that's a reasonable design, then I'd recommend we name this readability-alternative-tokens to be a bit more generic. (Note, we can always do the "don't use alternative tokens" implementation in a follow-up patch if you don't want to do that work immediately.)

clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.cpp

54–55

For C code, you could augment this FixIt with an include inserter, e.g., https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp#L112

clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.h

5–6

Should there be an NFC to go and update the other checks' licences too?

I think that'd be useful!

26

We can support C for this as well, can't we? iso646.h has been around since C99.

[• Quuxplusone]

clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.cpp

42–43

~ and ! are not binary operations, right?

clang-tools-extra/docs/clang-tidy/checks/readability-use-alternative-tokens.rst

7–8

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 & | ^ ~ << >> &= |= ~= <<= >>=.

50–53

"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.

59

s/logical/bitwise/

[cjdb]

• renames check to 'readability-alternative-tokens'
• adds section in documentation about configurability
• adds support for C source
• corrects whitespace handling in fixits

[cjdb]

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

133–134

Hrmm.... I'll do the rename now, but this might be better off as a later patch. I'd rather focus on getting what I have right (along with my teased extensions) and then work on the opposite direction. That will (probably) be an easy slip-in.

clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.cpp

42–43

This confused me too. binaryOperation also takes into account all overloaded ops.

54–55

Thanks! This was one of the blockers for getting me C support.

clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.h

26

I was having difficulty in my C test for this, wimped out, and put it into the "I'll do it later" basket. Lemme double check that's the case?

clang-tools-extra/docs/clang-tidy/checks/readability-use-alternative-tokens.rst

7–8

As agreed with Aaron above, I'll be making this configurable at a later date.

50–53

I think "program composition" is used enough in contemporary programming for readers to understand the text as it is.

[whisperity]

clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp

61 ↗

(On Diff #364307)

What's N? It's not immediately apparent for me from the callsites of this function.

70–71 ↗

(On Diff #364307)

Are we sure these will never return an invalid Loc, or dereference a null? Maybe it'd be worth to investigate, add an assertion (to help the people who might run static analysis on LLVM, if for nothing else), or an early return.

90–91 ↗

(On Diff #364307)

Same comment as in the UnaryOperator case.

124–125 ↗

(On Diff #364307)

Ditto. (Usually there might be issues if the location is coming from generated code or templates or macros or something... I fear this could be mostly prevalent in the user-defined operator case, i.e. this method.)

clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.h

20–21 ↗

(On Diff #364307)

Following from gone thread due to file rename.

In D107294#2923102, @cjdb wrote:

Not sure I'm following you here: are you suggesting I put the contents of my rst file in a comment here?

Not the entire RST, but the one-sentence or first-paragraph "pitch". For example, let's see bugprone-branch-clone's class's doc-comment:

/// A check for detecting if/else if/else chains where two or more branches are
/// Type I clones of each other (that is, they contain identical code), for
/// detecting switch statements where two or more consecutive branches are
/// Type I clones of each other, and for detecting conditional operators where
/// the true and false expressions are Type I clones of each other.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-branch-clone.html
class BranchCloneCheck : public ClangTidyCheck {

Or another one selected randomly, performance-no-automatic-move:

/// Finds local variables that cannot be automatically moved due to constness.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/performance-no-automatic-move.html
class NoAutomaticMoveCheck : public ClangTidyCheck {

So there is a one-paragraph summary of the check itself (it could be shorter than here...), and there is a text that's generated from a template (I think add-new-check.py sets the new check's files as such when you run it), which basically just links the upstream official website render of your check's documentation HTML.

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

133–134

(As someone who's had checkers on review for multiple years I've no hard feelings about the scheduling.)

But please do add a few FIXME: or TODO: or IDEA: or some similar comments to the code somewhere about the suggested follow-ups. (Just so they don't go away when this review is closed.)

clang-tools-extra/docs/clang-tidy/checks/readability-alternative-tokens.rst

6–7 ↗

(On Diff #364307)

I think (for now, until the check is improved/extended) this first paragraph / oneliner added to the doccomment in the header shall suffice.

It's only to ensure that the automatically generated documentation for the check's class has a comment from which documentation readers can look into what the check actually does (from a user's standpoint), by reaching this file.

[aaron.ballman]

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

133–134

Yeah, I wasn't trying to sign you up for the implementation effort, just making sure the name is somewhat more future-proofed. Adding a FIXME comment to the code to explain the potential next steps would be a good thing.

[cjdb]

• adds TODOs
• adds asserts
• removes "imported" SourceManager (apparently there was already a pointer in Result)
• fixes C++ documentation to match opening line of RST doc

[cjdb]

clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.h

20–21 ↗

(On Diff #364307)

Thanks for the comprehensive examples!

[cjdb]

Ping @aaron.ballman

[aaron.ballman]

clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp

51–54 ↗

(On Diff #364555)

69 ↗

(On Diff #364555)

Hrmm, I feel like std::isspace() is not going to properly handle all the Unicode whitespace, but I don't think we handle them all anyway as I notice the lexer is using isHorizontalWhitespace(), isVerticalWhitespace(), and isWhitespace() from CharInfo.h. Maybe we should use the same utility here just to match the lexer? (I don't feel strongly, just tickled my spidey senses.)

78–79 ↗

(On Diff #364555)

Are you planning to do this as part of this patch (if not, should this switch to FIXME)?

85 ↗

(On Diff #364555)

isLetter()? (or potentially isIdentifierHead() if we need to care about dollar-sign prefixed identifiers)

104 ↗

(On Diff #364555)

Same here as above.

111 ↗

(On Diff #364555)

Same here as above.

117–120 ↗

(On Diff #364555)

I think these can be replaced with a helper function along these lines:

void emitDiag(const SourceManager &SM, SourceLocation Loc, StringRef Op, StringRef OpDesc, int Lookahead) {
  diag(Loc, ("use '" + Op + "' for " + OpDesc).str()) <<
    createReplacement(SM, Loc, Op, Lookahead) << includeIso646(SM, Loc);
}

WDYT?

144–148 ↗

(On Diff #364555)

FIXMEs?

158–164 ↗

(On Diff #364555)

Nit: given that these are only used once, I'd rather just spell out the case labels longhand.

clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.h

35 ↗

(On Diff #364555)

clang-tools-extra/clang-tidy/readability/StrictConstCorrectness.h

9 ↗

(On Diff #364555)

Did you mean to include this file in this review?

clang-tools-extra/clang-tidy/readability/StrictConstCorrectness.kpp

1 ↗

(On Diff #364555)

Same for this one?

clang-tools-extra/docs/clang-tidy/checks/readability-alternative-tokens.rst

4 ↗

(On Diff #364555)

clang/test/Analysis/diagnostics/readability-strict-const-correctness.cpp

1 ↗

(On Diff #364555)

No idea how this managed to trigger a CI failure of line 1: fg: no job control :-D

[cor3ntin]

clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp

69 ↗

(On Diff #364555)

I just saw this comment randomly in my mail.
I agree we should not use isspace - it's also probably less efficient because of locale. And we don't want lexing to depend on the host locale.
And makes it easier to extend the set of supported whitespace if we ever do thatt

[cjdb]

clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp

85 ↗

(On Diff #364555)

Is this necessary in the given context? The reason I was confident in using isalpha is because we know this is an operator and the only values that this could possibly be are one of a, b, c, n, o, or x.

(cc @cor3ntin)

clang-tools-extra/clang-tidy/readability/StrictConstCorrectness.h

9 ↗

(On Diff #364555)

Lol nope. My bad.

clang/test/Analysis/diagnostics/readability-strict-const-correctness.cpp

1 ↗

(On Diff #364555)

How did these files even get into this branch!? Sorry for the noise.

[cor3ntin]

clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp

85 ↗

(On Diff #364555)

Definitively.
Both because trying to make assumption based on the current set of leading symbols in alternative token seems brittle to me.
But mostly because isalpha answers "is this byte interpreted in the encoding associated to the current locale a letter". in some scenario (clang running on zOS for example) this will never give the right answer,

I agree with Aaron: use isIdentifierHead()

But maybe I'd go further: Have you considered first checking for the opcode first, then the spelling?

[PiotrZSL]

Obsolete by D144522

[njames93]

The for Pipe/BitwiseOr issue. One heuristic could be:

• If it has any kind of template dependence don't diagnost
• If its a BinaryOperator, its safe to diagnose.
• If its a CxxOperatorCallExpr, the simplest case I can think of is to check the spelling of the operator (operator| vs operator bitor)

clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp

69 ↗

(On Diff #364555)

Is it not just wiser to always add the spaces and just let clang-format do its thing.

117–120 ↗

(On Diff #364555)

diag("use '%0' for %1") << Op << OpDesc << Fixes;

clang-tools-extra/clang-tidy/readability/CMakeLists.txt

add_clang_library(clangTidyReadabilityModule
   SimplifyBooleanExprCheck.cpp
   SimplifySubscriptExprCheck.cpp
   StaticAccessedThroughInstanceCheck.cpp
   StaticDefinitionInAnonymousNamespaceCheck.cpp
   StringCompareCheck.cpp
   SuspiciousCallArgumentCheck.cpp
   UniqueptrDeleteReleaseCheck.cpp
   UppercaseLiteralSuffixCheck.cpp
+  UseAlternativeTokensCheck.cpp
   UseAnyOfAllOfCheck.cpp
 
   LINK_LIBS
   clangTidy
   clangTidyUtils
 
   DEPENDS
   omp_gen

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "SimplifyBooleanExprCheck.h"
 #include "SimplifySubscriptExprCheck.h"
 #include "StaticAccessedThroughInstanceCheck.h"
 #include "StaticDefinitionInAnonymousNamespaceCheck.h"
 #include "StringCompareCheck.h"
 #include "SuspiciousCallArgumentCheck.h"
 #include "UniqueptrDeleteReleaseCheck.h"
 #include "UppercaseLiteralSuffixCheck.h"
+#include "UseAlternativeTokensCheck.h"
 #include "UseAnyOfAllOfCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace readability {
 
 class ReadabilityModule : public ClangTidyModule {
 public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<SimplifyBooleanExprCheck>(
         "readability-simplify-boolean-expr");
     CheckFactories.registerCheck<SuspiciousCallArgumentCheck>(
         "readability-suspicious-call-argument");
     CheckFactories.registerCheck<UniqueptrDeleteReleaseCheck>(
         "readability-uniqueptr-delete-release");
     CheckFactories.registerCheck<UppercaseLiteralSuffixCheck>(
         "readability-uppercase-literal-suffix");
+    CheckFactories.registerCheck<UseAlternativeTokensCheck>(
+        "readability-use-alternative-tokens");
     CheckFactories.registerCheck<UseAnyOfAllOfCheck>(
         "readability-use-anyofallof");
   }
 };
 
 // Register the ReadabilityModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<ReadabilityModule>
     X("readability-module", "Adds readability-related checks.");

clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.h

+//===--- UseAlternativeTokensCheck.h - clang-tidy--------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USEALTERNATIVETOKENSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USEALTERNATIVETOKENSCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include "clang/Basic/SourceManager.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+// Flags uses of symbol-based bitwise and logical operators.
+
+class UseAlternativeTokensCheck : public ClangTidyCheck {
+public:
+  using ClangTidyCheck::ClangTidyCheck;
+
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+                           Preprocessor *ModuleExpanderPP) override {
+    this->SM = &SM;
+  }
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const SourceManager *SM;
+
+  void checkSpelling(const UnaryOperator &);
+  void checkSpelling(const BinaryOperator &);
+  void checkSpelling(const CXXOperatorCallExpr &);
+};
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USEALTERNATIVETOKENSCHECK_H

clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.cpp

+//===--- UseAlternativeTokensCheck.cpp - clang-tidy------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseAlternativeTokensCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Expr.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/OperatorKinds.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/Lexer.h"
+
+#include <cstring>
+
+namespace clang {
+namespace tidy {
+namespace readability {
+namespace {
+AST_MATCHER(UnaryOperator, isBitwiseOrLogicalUnaryOp) {
+  return Node.getOpcode() == UnaryOperator::Opcode::UO_Not ||
+         Node.getOpcode() == UnaryOperator::Opcode::UO_LNot;
+}
+} // namespace
+
+using ast_matchers::binaryOperation;
+using ast_matchers::binaryOperator;
+using ast_matchers::hasAnyOperatorName;
+using ast_matchers::MatchFinder;
+using ast_matchers::unaryOperator;
+
+void UseAlternativeTokensCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(unaryOperator(isBitwiseOrLogicalUnaryOp()).bind("unary"),
+                     this);
+  // Finder->addMatcher(binaryOperator(isBitwiseOrLogicalBinaryOp()).bind("binary_builtin"),
+  // this);
+  Finder->addMatcher(
+      binaryOperation(hasAnyOperatorName("&&", "||", "!", "&", "|", "~", "^"))
+          .bind("operator"),
+      this);
+}
+
+void UseAlternativeTokensCheck::checkSpelling(const UnaryOperator &Op) {
+  SourceLocation Loc = Op.getOperatorLoc();
+  char First = *(SM->getCharacterData(Loc));
+  if (std::isalpha(First))
+    return;
+
+  bool IsLogicalNot = Op.getOpcode() == UnaryOperator::Opcode::UO_LNot;
+  auto Hint = FixItHint::CreateReplacement(Op.getOperatorLoc(),
+                                           IsLogicalNot ? "not " : "compl ");
+
+  diag(Loc, "use '%select{not|compl}0' for %select{logical|bitwise}0 negation")
+      << (IsLogicalNot ? 0 : 1) << Hint;
+}
+
+void UseAlternativeTokensCheck::checkSpelling(const BinaryOperator &Op) {
+  SourceLocation Loc = Op.getOperatorLoc();
+  char First = *(SM->getCharacterData(Loc));
+  if (std::isalpha(First))
+    return;
+
+  using Opcode = BinaryOperator::Opcode;
+  Opcode OC = Op.getOpcode();
+  switch (OC) {
+  case Opcode::BO_LAnd:
+    diag(Loc, "use 'and' for logical conjunctions")
+        << FixItHint::CreateReplacement(Loc, " and ");
+    break;
+  case Opcode::BO_And:
+    diag(Loc, "use 'bitand' for bitwise conjunctions")
+        << FixItHint::CreateReplacement(Loc, " bitand ");
+    break;
+  case Opcode::BO_Or:
+    diag(Loc, "use 'bitor' for bitwise disjunctions")
+        << FixItHint::CreateReplacement(Loc, " bitor ");
+    break;
+  case Opcode::BO_LOr:
+    diag(Loc, "use 'or' for logical disjunctions")
+        << FixItHint::CreateReplacement(Loc, " or ");
+    break;
+  case Opcode::BO_Xor:
+    diag(Loc, "use 'xor' for exclusive or")
+        << FixItHint::CreateReplacement(Loc, " xor ");
+    break;
+  default:
+    break;
+  }
+}
+
+void UseAlternativeTokensCheck::checkSpelling(const CXXOperatorCallExpr &Op) {
+  SourceLocation Loc = Op.getOperatorLoc();
+  char First = *(SM->getCharacterData(Loc));
+  if (std::isalpha(First))
+    return;
+
+  using Opcode = OverloadedOperatorKind;
+  constexpr Opcode And = Opcode::OO_AmpAmp;
+  constexpr Opcode Bitand = Opcode::OO_Amp;
+  constexpr Opcode Bitor = Opcode::OO_Pipe;
+  constexpr Opcode Compl = Opcode::OO_Tilde;
+  constexpr Opcode Not = Opcode::OO_Exclaim;
+  constexpr Opcode Or = Opcode::OO_PipePipe;
+  constexpr Opcode Xor = Opcode::OO_Caret;
+
+  Opcode OC = Op.getOperator();
+  switch (OC) {
+  case And:
+    diag(Loc, "use 'and' for logical conjunctions")
+        << FixItHint::CreateReplacement(Loc, " and ");
+    break;
+  case Bitand:
+    diag(Loc, "use 'bitand' for bitwise conjunctions")
+        << FixItHint::CreateReplacement(Loc, " bitand ");
+    break;
+  case Bitor:
+    diag(Loc, "use 'bitor' for bitwise disjunctions")
+        << FixItHint::CreateReplacement(Loc, " bitor ");
+    break;
+  case Compl:
+    diag(Loc, "use 'compl' for bitwise negation")
+        << FixItHint::CreateReplacement(Loc, "compl ");
+    break;
+  case Not:
+    diag(Loc, "use 'not' for logical negation")
+        << FixItHint::CreateReplacement(Loc, "not ");
+    break;
+  case Or:
+    diag(Loc, "use 'or' for logical disjunctions")
+        << FixItHint::CreateReplacement(Loc, " or ");
+    break;
+  case Xor:
+    diag(Loc, "use 'xor' for exclusive or")
+        << FixItHint::CreateReplacement(Loc, " xor ");
+    break;
+  default:
+    break;
+  }
+}
+
+void UseAlternativeTokensCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *Op = Result.Nodes.getNodeAs<UnaryOperator>("unary"))
+    return checkSpelling(*Op);
+
+  if (const auto *Op = Result.Nodes.getNodeAs<BinaryOperator>("operator"))
+    return checkSpelling(*Op);
+
+  if (const auto *Op = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("operator"))
+    return checkSpelling(*Op);
+}
+} // namespace readability
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/clang-tidy/checks/readability-use-alternative-tokens.rst

+.. title:: clang-tidy - readability-use-alternative-tokens
+
+readability-use-alternative-tokens
+==================================
+
+Finds uses of symbol-based logical and bitwise operators and recommends using
+alternative tokens instead.
+
+Although symbols are the mainstream tokens in syntaxes derived from C, spelling
+out the operators can be clearer. It more clearly expresses programmer intention
+by reducing barriers for interpretation, makes small operators stand out, makes
+similar tokens more visibly distinguishable, and reduces the chances for typos.
+
+This check doesn't yet account for program composition.
+
+.. code-block:: c++
+
+  // warning: use 'and' for logical conjunctions
+  x && y
+
+  // warning: use 'bitand' for bitwise conjunctions
+  x & y
+
+  // warning: use 'bitor' for bitwise disjunctions
+  x | y
+
+  // warning: use 'compl' for bitwise negation
+  ~x
+
+  // warning: use 'not' for logical negation
+  not x
+
+  // warning: use 'or' for logical disjunctions
+  x || y
+
+  // warning: use 'xor' for exclusive or
+  x ^ y

clang-tools-extra/test/clang-tidy/checkers/readability-use-alternative-tokens.cpp

+// RUN: %check_clang_tidy %s readability-use-alternative-tokens %t -- -- -I%S/Inputs
+
+// A note to the reader: the whitespace in this file is important: `true&&false`
+// is lexically three tokens, but `trueandfalse` is lexed as a single
+// identifier. This test needs to make sure that the fixit applies whitespace in
+// its change.
+
+// clang-format off
+
+void f1()
+{
+  (void)(true and true); // no warning
+  (void)(true&&false);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use 'and' for logical conjunctions
+  // CHECK-FIXES: true and false
+
+  (void)(1 bitand 2); // no warning
+  (void)(0&1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'bitand' for bitwise conjunctions
+  // CHECK-FIXES: 0 bitand 1
+
+  (void)(1 bitor 2); // no warning
+  (void)(0|1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'bitor' for bitwise disjunctions
+  // CHECK-FIXES: 0 bitor 1
+
+  (void)(compl 1); // no warning
+  (void)(~0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'compl' for bitwise negation
+  // CHECK-FIXES: compl 0
+
+  (void)(not true);
+  (void)(!false);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'not' for logical negation
+  // CHECK-FIXES: not false
+
+  (void)(true or true);
+  (void)(true||false);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use 'or' for logical disjunctions
+  // CHECK-FIXES: true or false
+
+  (void)(1 xor 0);
+  (void)(0^1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'xor' for exclusive or
+  // CHECK-FIXES: 0 xor 1
+}
+
+struct S {
+  S operator and(S) const;
+  S operator bitand(S) const;
+  S operator bitor(S) const;
+  S operator compl() const;
+  S operator not() const;
+  S operator or(S) const;
+  S operator xor(S) const;
+};
+
+void f5()
+{
+  S x;
+  S y;
+
+  (void)(x&&y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'and' for logical conjunctions
+  // CHECK-FIXES: x and y
+
+  (void)(x&y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'bitand' for bitwise conjunctions
+  // CHECK-FIXES: x bitand y
+
+  (void)(x|y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'bitor' for bitwise disjunctions
+  // CHECK-FIXES: x bitor y
+
+  (void)(~x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'compl' for bitwise negation
+  // CHECK-FIXES: compl x
+
+  (void)(!x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'not' for logical negation
+  // CHECK-FIXES: not x
+
+  (void)(x||y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'or' for logical disjunctions
+  // CHECK-FIXES: x or y
+
+  (void)(x^y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'xor' for exclusive or
+  // CHECK-FIXES: x xor y
+}

llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/readability/BUILD.gn

sources = [
     "SimplifyBooleanExprCheck.cpp",
     "SimplifySubscriptExprCheck.cpp",
     "StaticAccessedThroughInstanceCheck.cpp",
     "StaticDefinitionInAnonymousNamespaceCheck.cpp",
     "StringCompareCheck.cpp",
     "SuspiciousCallArgumentCheck.cpp",
     "UniqueptrDeleteReleaseCheck.cpp",
     "UppercaseLiteralSuffixCheck.cpp",
+    "UseAlternativeTokensCheck.cpp",
     "UseAnyOfAllOfCheck.cpp",
   ]
 }