[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL abandoned this revision.
PiotrZSL added a comment.

Obsolete by D144522 


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

https://reviews.llvm.org/D31308

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: xazax.hun.

As noted above, my concern with the current implementation is that the use of 
AST in this check seems to be superfluous. It should be enough to handle 
PPCallbacks and re-lex each file opened during compilation to find all 
alternative tokens.


https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added a comment.

Yep, I am also a fan of "and","or" and "not". The other tokens doesn't make it 
more readable imho


https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34
+
+  if (const auto *B = Result.Nodes.getNodeAs("binary")) {
+switch (B->getOpcode()) {

alexfh wrote:
> Eugene.Zelenko wrote:
> > aaron.ballman wrote:
> > > alexfh wrote:
> > > > aaron.ballman wrote:
> > > > > mgehre wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > alexfh wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > I think this would make more sense lifted into an AST matcher 
> > > > > > > > > -- there are bound to be a *lot* of binary operators, so 
> > > > > > > > > letting the matcher memoize things is likely to give better 
> > > > > > > > > performance.
> > > > > > > > Any reasons not to do this on the lexer level?
> > > > > > > Technical reasons? None.
> > > > > > > User-experience reasons? We wouldn't want this to be on by 
> > > > > > > default (I don't think) and we usually don't implement 
> > > > > > > off-by-default diagnostics in Clang. I think a case could be made 
> > > > > > > for doing it in the Lexer if the performance is really that bad 
> > > > > > > with a clang-tidy check and we couldn't improve it here, though.
> > > > > > Do I correctly understand that "doing this on lexer level" would 
> > > > > > mean to implement this as a warning directly into clang? If yes, 
> > > > > > would it be possible to generate fixits and have them possibly 
> > > > > > applied automatically (as it is the case for clang-tidy)?
> > > > > You are correct, that means implementing it as a warning in Clang. I 
> > > > > believe you can still generate those fixits from lexer-level 
> > > > > diagnostics, but have not verified it.
> > > > > 
> > > > > However, I don't think this diagnostic would be appropriate for Clang 
> > > > > because it would have to be off by default.
> > > > Actually, I was thinking about changing this clang-tidy check to 
> > > > analyze token stream somehow (probably by handling `PPCallbacks` to 
> > > > detect ranges that need to be re-lexed) instead of matching the AST. I 
> > > > didn't intend to propose a new Clang warning (but it looks like the 
> > > > wording was misleading).
> > > There is some value in that -- it means we could support C, for instance. 
> > > I'm not certain how easy or hard it would be, but suspect it's 
> > > reasonable. However, in C, there's still the problem of the include file 
> > > that introduces those macros. Do we have facilities to remove an include 
> > > in clang-tidy?
> > Yes, it may make sense in C, but parameter for map from macro name to 
> > operator is needed.
> > 
> > Product I'm working for, with long history, had alternative operator 
> > presentation implemented in C.
> Changing macro-based "alternative tokens" to the corresponding operators can 
> also be formulated in the terms of expanding a set of macros, which should 
> work with any LangOpts and arbitrary alternative operator names.
> 
> > However, in C, there's still the problem of the include file that 
> > introduces those macros. 
> 
> Sounds like a generic "remove unused includes" problem, since we need to 
> analyze whether the header is needed for anything else. In particular, even 
> if the use of the header is limited to the alternative representations of 
> operators, we can't remove the include until we replace all these macros. 
> Clang-tidy doesn't provide any support for transactional fixes and 
> dependencies between fixes.
> Sounds like a generic "remove unused includes" problem, since we need to 
> analyze whether the header is needed for anything else. In particular, even 
> if the use of the header is limited to the alternative representations of 
> operators, we can't remove the include until we replace all these macros. 
> Clang-tidy doesn't provide any support for transactional fixes and 
> dependencies between fixes.

That's a good point. I also don't think an unused include is sufficiently awful 
to block this.


https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34
+
+  if (const auto *B = Result.Nodes.getNodeAs("binary")) {
+switch (B->getOpcode()) {

Eugene.Zelenko wrote:
> aaron.ballman wrote:
> > alexfh wrote:
> > > aaron.ballman wrote:
> > > > mgehre wrote:
> > > > > aaron.ballman wrote:
> > > > > > alexfh wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > I think this would make more sense lifted into an AST matcher 
> > > > > > > > -- there are bound to be a *lot* of binary operators, so 
> > > > > > > > letting the matcher memoize things is likely to give better 
> > > > > > > > performance.
> > > > > > > Any reasons not to do this on the lexer level?
> > > > > > Technical reasons? None.
> > > > > > User-experience reasons? We wouldn't want this to be on by default 
> > > > > > (I don't think) and we usually don't implement off-by-default 
> > > > > > diagnostics in Clang. I think a case could be made for doing it in 
> > > > > > the Lexer if the performance is really that bad with a clang-tidy 
> > > > > > check and we couldn't improve it here, though.
> > > > > Do I correctly understand that "doing this on lexer level" would mean 
> > > > > to implement this as a warning directly into clang? If yes, would it 
> > > > > be possible to generate fixits and have them possibly applied 
> > > > > automatically (as it is the case for clang-tidy)?
> > > > You are correct, that means implementing it as a warning in Clang. I 
> > > > believe you can still generate those fixits from lexer-level 
> > > > diagnostics, but have not verified it.
> > > > 
> > > > However, I don't think this diagnostic would be appropriate for Clang 
> > > > because it would have to be off by default.
> > > Actually, I was thinking about changing this clang-tidy check to analyze 
> > > token stream somehow (probably by handling `PPCallbacks` to detect ranges 
> > > that need to be re-lexed) instead of matching the AST. I didn't intend to 
> > > propose a new Clang warning (but it looks like the wording was 
> > > misleading).
> > There is some value in that -- it means we could support C, for instance. 
> > I'm not certain how easy or hard it would be, but suspect it's reasonable. 
> > However, in C, there's still the problem of the include file that 
> > introduces those macros. Do we have facilities to remove an include in 
> > clang-tidy?
> Yes, it may make sense in C, but parameter for map from macro name to 
> operator is needed.
> 
> Product I'm working for, with long history, had alternative operator 
> presentation implemented in C.
Changing macro-based "alternative tokens" to the corresponding operators can 
also be formulated in the terms of expanding a set of macros, which should work 
with any LangOpts and arbitrary alternative operator names.

> However, in C, there's still the problem of the include file that introduces 
> those macros. 

Sounds like a generic "remove unused includes" problem, since we need to 
analyze whether the header is needed for anything else. In particular, even if 
the use of the header is limited to the alternative representations of 
operators, we can't remove the include until we replace all these macros. 
Clang-tidy doesn't provide any support for transactional fixes and dependencies 
between fixes.


https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34
+
+  if (const auto *B = Result.Nodes.getNodeAs("binary")) {
+switch (B->getOpcode()) {

aaron.ballman wrote:
> alexfh wrote:
> > aaron.ballman wrote:
> > > mgehre wrote:
> > > > aaron.ballman wrote:
> > > > > alexfh wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > I think this would make more sense lifted into an AST matcher -- 
> > > > > > > there are bound to be a *lot* of binary operators, so letting the 
> > > > > > > matcher memoize things is likely to give better performance.
> > > > > > Any reasons not to do this on the lexer level?
> > > > > Technical reasons? None.
> > > > > User-experience reasons? We wouldn't want this to be on by default (I 
> > > > > don't think) and we usually don't implement off-by-default 
> > > > > diagnostics in Clang. I think a case could be made for doing it in 
> > > > > the Lexer if the performance is really that bad with a clang-tidy 
> > > > > check and we couldn't improve it here, though.
> > > > Do I correctly understand that "doing this on lexer level" would mean 
> > > > to implement this as a warning directly into clang? If yes, would it be 
> > > > possible to generate fixits and have them possibly applied 
> > > > automatically (as it is the case for clang-tidy)?
> > > You are correct, that means implementing it as a warning in Clang. I 
> > > believe you can still generate those fixits from lexer-level diagnostics, 
> > > but have not verified it.
> > > 
> > > However, I don't think this diagnostic would be appropriate for Clang 
> > > because it would have to be off by default.
> > Actually, I was thinking about changing this clang-tidy check to analyze 
> > token stream somehow (probably by handling `PPCallbacks` to detect ranges 
> > that need to be re-lexed) instead of matching the AST. I didn't intend to 
> > propose a new Clang warning (but it looks like the wording was misleading).
> There is some value in that -- it means we could support C, for instance. I'm 
> not certain how easy or hard it would be, but suspect it's reasonable. 
> However, in C, there's still the problem of the include file that introduces 
> those macros. Do we have facilities to remove an include in clang-tidy?
Yes, it may make sense in C, but parameter for map from macro name to operator 
is needed.

Product I'm working for, with long history, had alternative operator 
presentation implemented in C.


https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34
+
+  if (const auto *B = Result.Nodes.getNodeAs("binary")) {
+switch (B->getOpcode()) {

alexfh wrote:
> aaron.ballman wrote:
> > mgehre wrote:
> > > aaron.ballman wrote:
> > > > alexfh wrote:
> > > > > aaron.ballman wrote:
> > > > > > I think this would make more sense lifted into an AST matcher -- 
> > > > > > there are bound to be a *lot* of binary operators, so letting the 
> > > > > > matcher memoize things is likely to give better performance.
> > > > > Any reasons not to do this on the lexer level?
> > > > Technical reasons? None.
> > > > User-experience reasons? We wouldn't want this to be on by default (I 
> > > > don't think) and we usually don't implement off-by-default diagnostics 
> > > > in Clang. I think a case could be made for doing it in the Lexer if the 
> > > > performance is really that bad with a clang-tidy check and we couldn't 
> > > > improve it here, though.
> > > Do I correctly understand that "doing this on lexer level" would mean to 
> > > implement this as a warning directly into clang? If yes, would it be 
> > > possible to generate fixits and have them possibly applied automatically 
> > > (as it is the case for clang-tidy)?
> > You are correct, that means implementing it as a warning in Clang. I 
> > believe you can still generate those fixits from lexer-level diagnostics, 
> > but have not verified it.
> > 
> > However, I don't think this diagnostic would be appropriate for Clang 
> > because it would have to be off by default.
> Actually, I was thinking about changing this clang-tidy check to analyze 
> token stream somehow (probably by handling `PPCallbacks` to detect ranges 
> that need to be re-lexed) instead of matching the AST. I didn't intend to 
> propose a new Clang warning (but it looks like the wording was misleading).
There is some value in that -- it means we could support C, for instance. I'm 
not certain how easy or hard it would be, but suspect it's reasonable. However, 
in C, there's still the problem of the include file that introduces those 
macros. Do we have facilities to remove an include in clang-tidy?



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:68
+  if (PrimarySpelling != Spelling) {
+diag(OpLoc, "operator uses alternative spelling")
+<< FixItHint::CreateReplacement(TokenRange, PrimarySpelling);

aaron.ballman wrote:
> mgehre wrote:
> > aaron.ballman wrote:
> > > This diagnostic doesn't help the user to understand what's wrong with 
> > > their code (especially in the presence of multiple operators). Perhaps 
> > > "'%0' is an alternative token spelling; consider using '%1'"
> > > 
> > > It would be nice if we could say "consider using %1 for ", but 
> > > I'm really not certain why we would diagnose this code in the first place 
> > > (it's purely a matter of stylistic choice, as I understand it).
> > The main rational for this check is to enforce consistency and thus make it 
> > easier to read and comprehend the code.
> > I agree with your proposed diagnostics.
> I think that enforcing consistency is a good rationale for having the check, 
> but would second the suggestion that this check have an option to enforce the 
> consistency one way or the other.
> 
> Then the diagnostic can be:
> 
> "'%0' is %select{an alternative token|a primary token}2; consider using '%1' 
> for consistency"
The diagnostic is improved, but there's still no way to go the opposite 
direction (from primary to alternative).


https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 93519.
mgehre added a comment.

only check C++ code; only match operators that can have alternative 
representations


https://reviews.llvm.org/D31308

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/OperatorsRepresentationCheck.cpp
  clang-tidy/readability/OperatorsRepresentationCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-operators-representation.rst
  test/clang-tidy/readability-operators-representation.cpp

Index: test/clang-tidy/readability-operators-representation.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-operators-representation.cpp
@@ -0,0 +1,63 @@
+// RUN: %check_clang_tidy %s readability-operators-representation %t
+
+void f() {
+  bool a, b, c;
+
+  c = a and b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: 'and' is an alternative token spelling; consider using '&&' [readability-operators-representation]
+  // CHECK-FIXES: c = a && b;
+  c and_eq a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'and_eq' is an alternative
+  // CHECK-FIXES: c &= a;
+  c = a bitand b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: 'bitand' is an alternative
+  // CHECK-FIXES: c = a & b;
+  c = a bitor b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: 'bitor' is an alternative
+  // CHECK-FIXES: c = a | b;
+  c = compl a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'compl' is an alternative
+  // CHECK-FIXES: c = ~ a;
+  c = not a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'not' is an alternative
+  // CHECK-FIXES: c = ! a;
+  c = a not_eq b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: 'not_eq' is an alternative
+  // CHECK-FIXES: c = a != b;
+  c = a or b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: 'or' is an alternative
+  // CHECK-FIXES: c = a || b;
+  c or_eq a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'or_eq' is an alternative
+  // CHECK-FIXES: c |= a;
+  c = a xor b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: 'xor' is an alternative
+  // CHECK-FIXES: c = a ^ b;
+  c xor_eq a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'xor_eq' is an alternative
+  // CHECK-FIXES: c ^= a;
+
+#define M a xor
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 'xor' is an alternative
+  // CHECK-FIXES: #define M a ^
+  c = M b;
+
+  int arr[2];
+  for (int i : arr) // OK (Here is an implicit != operator.)
+;
+
+  auto ptr =  // OK
+  auto i = -1;   // OK
+  c = a && b;// OK
+  c &= a;// OK
+  c = !a;// OK
+}
+
+struct S {
+  friend S  and(const S &, const S &);
+};
+
+int g() {
+  S s1, s2;
+  S s3 = s1 and s2; // OK
+}
Index: docs/clang-tidy/checks/readability-operators-representation.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-operators-representation.rst
@@ -0,0 +1,23 @@
+.. title:: clang-tidy - readability-operators-representation
+
+readability-operators-representation
+
+
+Flags (and replaces) the alternative tokens for binary and unary operators by
+their primary ones for consistency.
+
+=== ===
+Primary Alternative
+=== ===
+``&&``  ``and``
+``&=``  ``and_eq``
+``&``   ``bitand``
+``|``   ``bitor``
+``~``   ``compl``
+``!``   ``not``
+``!=``  ``not_eq``
+``||``  ``or``
+``|=``  ``or_eq``
+``^``   ``xor``
+``^=``  ``xor_eq``
+=== ===
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -145,6 +145,7 @@
readability-misleading-indentation
readability-misplaced-array-index
readability-named-parameter
+   readability-operators-representation
readability-non-const-parameter
readability-redundant-control-flow
readability-redundant-declaration
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -72,6 +72,12 @@
 
   Finds misleading indentation where braces should be introduced or the code should be reformatted.
 
+- New `readability-operators-representation
+  `_ check
+
+  Flags (and replaces) the alternative tokens for binary and unary operators,
+  such as ``not`` (for ``!``) and ``or`` (for ``||``).
+
 - Added `ParameterThreshold` to `readability-function-size`.
 
   Finds functions that have more then `ParameterThreshold` parameters and emits a warning.
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -24,6 +24,7 @@
 #include "MisplacedArrayIndexCheck.h"
 #include 

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34
+
+  if (const auto *B = Result.Nodes.getNodeAs("binary")) {
+switch (B->getOpcode()) {

aaron.ballman wrote:
> mgehre wrote:
> > aaron.ballman wrote:
> > > alexfh wrote:
> > > > aaron.ballman wrote:
> > > > > I think this would make more sense lifted into an AST matcher -- 
> > > > > there are bound to be a *lot* of binary operators, so letting the 
> > > > > matcher memoize things is likely to give better performance.
> > > > Any reasons not to do this on the lexer level?
> > > Technical reasons? None.
> > > User-experience reasons? We wouldn't want this to be on by default (I 
> > > don't think) and we usually don't implement off-by-default diagnostics in 
> > > Clang. I think a case could be made for doing it in the Lexer if the 
> > > performance is really that bad with a clang-tidy check and we couldn't 
> > > improve it here, though.
> > Do I correctly understand that "doing this on lexer level" would mean to 
> > implement this as a warning directly into clang? If yes, would it be 
> > possible to generate fixits and have them possibly applied automatically 
> > (as it is the case for clang-tidy)?
> You are correct, that means implementing it as a warning in Clang. I believe 
> you can still generate those fixits from lexer-level diagnostics, but have 
> not verified it.
> 
> However, I don't think this diagnostic would be appropriate for Clang because 
> it would have to be off by default.
Actually, I was thinking about changing this clang-tidy check to analyze token 
stream somehow (probably by handling `PPCallbacks` to detect ranges that need 
to be re-lexed) instead of matching the AST. I didn't intend to propose a new 
Clang warning (but it looks like the wording was misleading).


https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34
+
+  if (const auto *B = Result.Nodes.getNodeAs("binary")) {
+switch (B->getOpcode()) {

mgehre wrote:
> aaron.ballman wrote:
> > alexfh wrote:
> > > aaron.ballman wrote:
> > > > I think this would make more sense lifted into an AST matcher -- there 
> > > > are bound to be a *lot* of binary operators, so letting the matcher 
> > > > memoize things is likely to give better performance.
> > > Any reasons not to do this on the lexer level?
> > Technical reasons? None.
> > User-experience reasons? We wouldn't want this to be on by default (I don't 
> > think) and we usually don't implement off-by-default diagnostics in Clang. 
> > I think a case could be made for doing it in the Lexer if the performance 
> > is really that bad with a clang-tidy check and we couldn't improve it here, 
> > though.
> Do I correctly understand that "doing this on lexer level" would mean to 
> implement this as a warning directly into clang? If yes, would it be possible 
> to generate fixits and have them possibly applied automatically (as it is the 
> case for clang-tidy)?
You are correct, that means implementing it as a warning in Clang. I believe 
you can still generate those fixits from lexer-level diagnostics, but have not 
verified it.

However, I don't think this diagnostic would be appropriate for Clang because 
it would have to be off by default.



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:68
+  if (PrimarySpelling != Spelling) {
+diag(OpLoc, "operator uses alternative spelling")
+<< FixItHint::CreateReplacement(TokenRange, PrimarySpelling);

mgehre wrote:
> aaron.ballman wrote:
> > This diagnostic doesn't help the user to understand what's wrong with their 
> > code (especially in the presence of multiple operators). Perhaps "'%0' is 
> > an alternative token spelling; consider using '%1'"
> > 
> > It would be nice if we could say "consider using %1 for ", but I'm 
> > really not certain why we would diagnose this code in the first place (it's 
> > purely a matter of stylistic choice, as I understand it).
> The main rational for this check is to enforce consistency and thus make it 
> easier to read and comprehend the code.
> I agree with your proposed diagnostics.
I think that enforcing consistency is a good rationale for having the check, 
but would second the suggestion that this check have an option to enforce the 
consistency one way or the other.

Then the diagnostic can be:

"'%0' is %select{an alternative token|a primary token}2; consider using '%1' 
for consistency"



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:22
+void OperatorsRepresentationCheck::registerMatchers(MatchFinder *Finder) {
+  // We ignore implicit != operators in C++11 range-based for loops.
+  Finder->addMatcher(binaryOperator(unless(hasLHS(ignoringImpCasts(

I think that this check should only be performed in C++. In C, the alternative 
token spellings are implemented as macros and would require additional work to 
support.


https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 93309.
mgehre added a comment.

Improved diagnostic; updated documentation; added test for overloaded operator


https://reviews.llvm.org/D31308

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/OperatorsRepresentationCheck.cpp
  clang-tidy/readability/OperatorsRepresentationCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-operators-representation.rst
  test/clang-tidy/readability-operators-representation.cpp

Index: test/clang-tidy/readability-operators-representation.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-operators-representation.cpp
@@ -0,0 +1,63 @@
+// RUN: %check_clang_tidy %s readability-operators-representation %t
+
+void f() {
+  bool a, b, c;
+
+  c = a and b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: 'and' is an alternative token spelling; consider using '&&' [readability-operators-representation]
+  // CHECK-FIXES: c = a && b;
+  c and_eq a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'and_eq' is an alternative
+  // CHECK-FIXES: c &= a;
+  c = a bitand b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: 'bitand' is an alternative
+  // CHECK-FIXES: c = a & b;
+  c = a bitor b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: 'bitor' is an alternative
+  // CHECK-FIXES: c = a | b;
+  c = compl a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'compl' is an alternative
+  // CHECK-FIXES: c = ~ a;
+  c = not a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'not' is an alternative
+  // CHECK-FIXES: c = ! a;
+  c = a not_eq b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: 'not_eq' is an alternative
+  // CHECK-FIXES: c = a != b;
+  c = a or b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: 'or' is an alternative
+  // CHECK-FIXES: c = a || b;
+  c or_eq a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'or_eq' is an alternative
+  // CHECK-FIXES: c |= a;
+  c = a xor b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: 'xor' is an alternative
+  // CHECK-FIXES: c = a ^ b;
+  c xor_eq a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'xor_eq' is an alternative
+  // CHECK-FIXES: c ^= a;
+
+#define M a xor
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 'xor' is an alternative
+  // CHECK-FIXES: #define M a ^
+  c = M b;
+
+  int arr[2];
+  for (int i : arr) // OK (Here is an implicit != operator.)
+;
+
+  auto ptr =  // OK
+  auto i = -1;   // OK
+  c = a && b;// OK
+  c &= a;// OK
+  c = !a;// OK
+}
+
+struct S {
+  friend S& operator and(const S &, const S &);
+};
+
+int g() {
+  S s1, s2;
+  S s3 = s1 and s2; // OK
+}
Index: docs/clang-tidy/checks/readability-operators-representation.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-operators-representation.rst
@@ -0,0 +1,23 @@
+.. title:: clang-tidy - readability-operators-representation
+
+readability-operators-representation
+
+
+Flags (and replaces) the alternative tokens for binary and unary operators by
+their primary ones for consistency.
+
+=== ===
+Primary Alternative
+=== ===
+``&&``  ``and``
+``&=``  ``and_eq``
+``&``   ``bitand``
+``|``   ``bitor``
+``~``   ``compl``
+``!``   ``not``
+``!=``  ``not_eq``
+``||``  ``or``
+``|=``  ``or_eq``
+``^``   ``xor``
+``^=``  ``xor_eq``
+=== ===
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -145,6 +145,7 @@
readability-misleading-indentation
readability-misplaced-array-index
readability-named-parameter
+   readability-operators-representation
readability-non-const-parameter
readability-redundant-control-flow
readability-redundant-declaration
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -72,6 +72,12 @@
 
   Finds misleading indentation where braces should be introduced or the code should be reformatted.
 
+- New `readability-operators-representation
+  `_ check
+
+  Flags (and replaces) the alternative tokens for binary and unary operators,
+  such as ``not`` (for ``!``) and ``or`` (for ``||``).
+
 - Added `ParameterThreshold` to `readability-function-size`.
 
   Finds functions that have more then `ParameterThreshold` parameters and emits a warning.
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -23,6 +23,7 @@
 #include "MisleadingIndentationCheck.h"
 

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added inline comments.



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34
+
+  if (const auto *B = Result.Nodes.getNodeAs("binary")) {
+switch (B->getOpcode()) {

aaron.ballman wrote:
> alexfh wrote:
> > aaron.ballman wrote:
> > > I think this would make more sense lifted into an AST matcher -- there 
> > > are bound to be a *lot* of binary operators, so letting the matcher 
> > > memoize things is likely to give better performance.
> > Any reasons not to do this on the lexer level?
> Technical reasons? None.
> User-experience reasons? We wouldn't want this to be on by default (I don't 
> think) and we usually don't implement off-by-default diagnostics in Clang. I 
> think a case could be made for doing it in the Lexer if the performance is 
> really that bad with a clang-tidy check and we couldn't improve it here, 
> though.
Do I correctly understand that "doing this on lexer level" would mean to 
implement this as a warning directly into clang? If yes, would it be possible 
to generate fixits and have them possibly applied automatically (as it is the 
case for clang-tidy)?



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:68
+  if (PrimarySpelling != Spelling) {
+diag(OpLoc, "operator uses alternative spelling")
+<< FixItHint::CreateReplacement(TokenRange, PrimarySpelling);

aaron.ballman wrote:
> This diagnostic doesn't help the user to understand what's wrong with their 
> code (especially in the presence of multiple operators). Perhaps "'%0' is an 
> alternative token spelling; consider using '%1'"
> 
> It would be nice if we could say "consider using %1 for ", but I'm 
> really not certain why we would diagnose this code in the first place (it's 
> purely a matter of stylistic choice, as I understand it).
The main rational for this check is to enforce consistency and thus make it 
easier to read and comprehend the code.
I agree with your proposed diagnostics.


https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34
+
+  if (const auto *B = Result.Nodes.getNodeAs("binary")) {
+switch (B->getOpcode()) {

alexfh wrote:
> aaron.ballman wrote:
> > I think this would make more sense lifted into an AST matcher -- there are 
> > bound to be a *lot* of binary operators, so letting the matcher memoize 
> > things is likely to give better performance.
> Any reasons not to do this on the lexer level?
Technical reasons? None.
User-experience reasons? We wouldn't want this to be on by default (I don't 
think) and we usually don't implement off-by-default diagnostics in Clang. I 
think a case could be made for doing it in the Lexer if the performance is 
really that bad with a clang-tidy check and we couldn't improve it here, though.


https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34
+
+  if (const auto *B = Result.Nodes.getNodeAs("binary")) {
+switch (B->getOpcode()) {

aaron.ballman wrote:
> I think this would make more sense lifted into an AST matcher -- there are 
> bound to be a *lot* of binary operators, so letting the matcher memoize 
> things is likely to give better performance.
Any reasons not to do this on the lexer level?


https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34
+
+  if (const auto *B = Result.Nodes.getNodeAs("binary")) {
+switch (B->getOpcode()) {

I think this would make more sense lifted into an AST matcher -- there are 
bound to be a *lot* of binary operators, so letting the matcher memoize things 
is likely to give better performance.



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:68
+  if (PrimarySpelling != Spelling) {
+diag(OpLoc, "operator uses alternative spelling")
+<< FixItHint::CreateReplacement(TokenRange, PrimarySpelling);

This diagnostic doesn't help the user to understand what's wrong with their 
code (especially in the presence of multiple operators). Perhaps "'%0' is an 
alternative token spelling; consider using '%1'"

It would be nice if we could say "consider using %1 for ", but I'm 
really not certain why we would diagnose this code in the first place (it's 
purely a matter of stylistic choice, as I understand it).



Comment at: docs/clang-tidy/checks/readability-operators-representation.rst:4
+readability-operators-representation
+=
+

This underline is going to cause Sphinx diagnostics.



Comment at: docs/clang-tidy/checks/readability-operators-representation.rst:8
+such as ``not`` (for ``!``), ``bitand`` (for ``&``), ``or`` (for ``||``) or 
``not_eq``
+(for ``!=``).

Why would someone want to do this? You should at least touch on that in the 
documentation.

Also, this doesn't read very clearly to me. Perhaps it would be better as a 
table with two columns, the first specifying the alternative token spelling and 
the second specifying the replacement (with suitable headings),



Comment at: test/clang-tidy/readability-operators-representation.cpp:54
+  c = !a;// OK
+}

Please add a test where a class overloads the operators. For special fun:
```
struct S {
  friend S& operator and(const S , const S );
};

int main() {
  S s1, s2;
  S s3 = s1 and s2;
}
```
I'm not convinced the correct answer here is to tell the user to use a spelling 
that isn't used by the class definition...


https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 93057.
mgehre added a comment.

Rename check to readability-operators-representation


https://reviews.llvm.org/D31308

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/OperatorsRepresentationCheck.cpp
  clang-tidy/readability/OperatorsRepresentationCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-operators-representation.rst
  test/clang-tidy/readability-operators-representation.cpp

Index: test/clang-tidy/readability-operators-representation.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-operators-representation.cpp
@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s readability-operators-representation %t
+
+void f() {
+  bool a, b, c;
+
+  c = a and b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operator uses alternative spelling [readability-operators-representation]
+  // CHECK-FIXES: c = a && b;
+  c and_eq a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operator
+  // CHECK-FIXES: c &= a;
+  c = a bitand b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operator
+  // CHECK-FIXES: c = a & b;
+  c = a bitor b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operator
+  // CHECK-FIXES: c = a | b;
+  c = compl a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: operator
+  // CHECK-FIXES: c = ~ a;
+  c = not a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: operator
+  // CHECK-FIXES: c = ! a;
+  c = a not_eq b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operator
+  // CHECK-FIXES: c = a != b;
+  c = a or b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operator
+  // CHECK-FIXES: c = a || b;
+  c or_eq a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operator
+  // CHECK-FIXES: c |= a;
+  c = a xor b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operator
+  // CHECK-FIXES: c = a ^ b;
+  c xor_eq a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operator
+  // CHECK-FIXES: c ^= a;
+
+#define M a xor
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: operator
+  // CHECK-FIXES: #define M a ^
+  c = M b;
+
+  int arr[2];
+  for (int i : arr) // OK (Here is an implicit != operator.)
+;
+
+  auto ptr =  // OK
+  auto i = -1;   // OK
+  c = a && b;// OK
+  c &= a;// OK
+  c = !a;// OK
+}
Index: docs/clang-tidy/checks/readability-operators-representation.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-operators-representation.rst
@@ -0,0 +1,8 @@
+.. title:: clang-tidy - readability-operators-representation
+
+readability-operators-representation
+=
+
+Flags (and replaces) the alternative tokens for binary and unary operators,
+such as ``not`` (for ``!``), ``bitand`` (for ``&``), ``or`` (for ``||``) or ``not_eq``
+(for ``!=``).
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -145,6 +145,7 @@
readability-misleading-indentation
readability-misplaced-array-index
readability-named-parameter
+   readability-operators-representation
readability-non-const-parameter
readability-redundant-control-flow
readability-redundant-declaration
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -72,6 +72,12 @@
 
   Finds misleading indentation where braces should be introduced or the code should be reformatted.
 
+- New `readability-operators-representation
+  `_ check
+
+  Flags (and replaces) the alternative tokens for binary and unary operators,
+  such as ``not`` (for ``!``) and ``or`` (for ``||``).
+
 - Added `ParameterThreshold` to `readability-function-size`.
 
   Finds functions that have more then `ParameterThreshold` parameters and emits a warning.
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -23,6 +23,7 @@
 #include "MisleadingIndentationCheck.h"
 #include "MisplacedArrayIndexCheck.h"
 #include "NamedParameterCheck.h"
+#include "OperatorsRepresentationCheck.h"
 #include "NonConstParameterCheck.h"
 #include "RedundantControlFlowCheck.h"
 #include "RedundantDeclarationCheck.h"
@@ -66,6 +67,8 @@
 "readability-misleading-indentation");
 CheckFactories.registerCheck(
 "readability-misplaced-array-index");
+CheckFactories.registerCheck(
+"readability-operators-representation");
 CheckFactories.registerCheck(
 "readability-redundant-function-ptr-dereference");
 

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Gonzalo BG via Phabricator via cfe-commits]

gnzlbg added a comment.

Thanks for working on this!

> I'm not sure if it would be helpful to have this check in both ways. I did a 
> code search for "not_eq", "bitand" and "and_eq" on github, and their usage 
> seems to be a clear minority.

I actually was requesting the opposite version of this (but suggesting to 
implement both), because for me "if (!something)" is much harder to read than 
"if (not something)" (I am a bit blind).

> So I would propose to keep the features as-is for now, change the name to 
> readability-operators-representation, and then later (someone else?) might 
> also add an option for making this work the other way around. Would that be 
> ok for you?

Sounds good to me. This solves half of the problem, and I agree with you that 
more people will benefit from this check than from the opposite check. Thanks 
again for working on this!


Repository:
  rL LLVM

https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

FWIW, I'm pretty sure this can and should be done on the lexer level - it will 
be faster and more universal.


Repository:
  rL LLVM

https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

> So I would propose to keep the features as-is for now,
>  change the name to readability-operators-representation, and then later 
> (someone else?) might also add an option
>  for making this work the other way around. Would that be ok for you?

Fine by me.


Repository:
  rL LLVM

https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In https://reviews.llvm.org/D31308#709709, @mgehre wrote:

> Thanks for your feedback, Eugene!
>
> I'm not sure if it would be helpful to have this check in both ways. I did a 
> code search for "not_eq", "bitand" and "and_eq"
>  on github, and their usage seems to be a clear minority.
>
> So I would propose to keep the features as-is for now,
>  change the name to readability-operators-representation, and then later 
> (someone else?) might also add an option
>  for making this work the other way around. Would that be ok for you?
>
> Were you refering to https://reviews.llvm.org/D25195 ("[lit] Fix FormatError 
> on individual test timeout")?


Nope, "PR" notation is used to reference bugs ("Problem Reports", PRobably): 
http://llvm.org/PR25195


Repository:
  rL LLVM

https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

Thanks for your feedback, Eugene!

I'm not sure if it would be helpful to have this check in both ways. I did a 
code search for "not_eq", "bitand" and "and_eq"
on github, and their usage seems to be a clear minority.

So I would propose to keep the features as-is for now,
change the name to readability-operators-representation, and then later 
(someone else?) might also add an option
for making this work the other way around. Would that be ok for you?

Were you refering to https://reviews.llvm.org/D25195 ("[lit] Fix FormatError on 
individual test timeout")?


Repository:
  rL LLVM

https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

I think will be good idea to make this check working in both ways (standard or 
alternative operator representations), depending on option.

Probably readability-operators-representation will be better name for check.

See also PR25195.


https://reviews.llvm.org/D31308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Matthias Gehre via Phabricator via cfe-commits]

mgehre created this revision.
Herald added a subscriber: mgorny.

Flags (and replaces) the alternative tokens for binary and unary
operators, such as ``not`` (for ``!``), ``bitand`` (for ``&``), ``or`` (for 
``||``)
or ``not_eq`` (for ``!=``).


https://reviews.llvm.org/D31308

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/NoAlternativeTokensCheck.cpp
  clang-tidy/readability/NoAlternativeTokensCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-no-alternative-tokens.rst
  test/clang-tidy/readability-no-alternative-tokens.cpp

Index: test/clang-tidy/readability-no-alternative-tokens.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-no-alternative-tokens.cpp
@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s readability-no-alternative-tokens %t
+
+void f() {
+  bool a, b, c;
+
+  c = a and b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operator uses alternative spelling [readability-no-alternative-tokens]
+  // CHECK-FIXES: c = a && b;
+  c and_eq a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operator
+  // CHECK-FIXES: c &= a;
+  c = a bitand b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operator
+  // CHECK-FIXES: c = a & b;
+  c = a bitor b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operator
+  // CHECK-FIXES: c = a | b;
+  c = compl a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: operator
+  // CHECK-FIXES: c = ~ a;
+  c = not a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: operator
+  // CHECK-FIXES: c = ! a;
+  c = a not_eq b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operator
+  // CHECK-FIXES: c = a != b;
+  c = a or b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operator
+  // CHECK-FIXES: c = a || b;
+  c or_eq a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operator
+  // CHECK-FIXES: c |= a;
+  c = a xor b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operator
+  // CHECK-FIXES: c = a ^ b;
+  c xor_eq a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operator
+  // CHECK-FIXES: c ^= a;
+
+#define M a xor
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: operator
+  // CHECK-FIXES: #define M a ^
+  c = M b;
+
+  int arr[2];
+  for (int i : arr) // OK (Here is an implicit != operator.)
+;
+
+  auto ptr =  // OK
+  auto i = -1;   // OK
+  c = a && b;// OK
+  c &= a;// OK
+  c = !a;// OK
+}
Index: docs/clang-tidy/checks/readability-no-alternative-tokens.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-no-alternative-tokens.rst
@@ -0,0 +1,8 @@
+.. title:: clang-tidy - readability-no-alternative-tokens
+
+readability-no-alternative-tokens
+=
+
+Flags (and replaces) the alternative tokens for binary and unary operators,
+such as ``not`` (for ``!``), ``bitand`` (for ``&``), ``or`` (for ``||``) or ``not_eq``
+(for ``!=``).
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -144,6 +144,7 @@
readability-misleading-indentation
readability-misplaced-array-index
readability-named-parameter
+   readability-no-alternative-tokens
readability-non-const-parameter
readability-redundant-control-flow
readability-redundant-declaration
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -72,6 +72,12 @@
 
   Finds misleading indentation where braces should be introduced or the code should be reformatted.
 
+- New `readability-no-alternative-tokens
+  `_ check
+
+  Flags (and replaces) the alternative tokens for binary and unary operators,
+  such as ``not`` (for ``!``) and ``or`` (for ``||``).
+
 - Added `ParameterThreshold` to `readability-function-size`.
 
   Finds functions that have more then `ParameterThreshold` parameters and emits a warning.
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -23,6 +23,7 @@
 #include "MisleadingIndentationCheck.h"
 #include "MisplacedArrayIndexCheck.h"
 #include "NamedParameterCheck.h"
+#include "NoAlternativeTokensCheck.h"
 #include "NonConstParameterCheck.h"
 #include "RedundantControlFlowCheck.h"
 #include "RedundantDeclarationCheck.h"
@@ -66,6 +67,8 @@
 "readability-misleading-indentation");
 CheckFactories.registerCheck(
 "readability-misplaced-array-index");
+CheckFactories.registerCheck(
+"readability-no-alternative-tokens");
 CheckFactories.registerCheck(