cjdb created this revision.
cjdb added a reviewer: aaron.ballman.
cjdb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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 fixit
- add a way for the compiler to detect when the pipe operator (`|`) is not
being used for a bitwise-or operation
- possibly extend this warning beyond C++
Depends on D107291 <https://reviews.llvm.org/D107291>.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D107294
Files:
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/lib/Parse/ParseExpr.cpp
clang/test/Parser/warn-use-alternative-tokens.c
clang/test/Parser/warn-use-alternative-tokens.cpp
clang/test/Sema/vector-gcc-compat.cpp
Index: clang/test/Sema/vector-gcc-compat.cpp
===================================================================
--- clang/test/Sema/vector-gcc-compat.cpp
+++ clang/test/Sema/vector-gcc-compat.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -fsyntax-only -Weverything -Wno-unused-but-set-variable -std=c++11 -triple x86_64-apple-darwin10
+// RUN: %clang_cc1 %s -verify -fsyntax-only -Weverything -Wno-unused-but-set-variable -Wno-use-alternative-tokens-in-expressions -std=c++11 -triple x86_64-apple-darwin10
// Test the compatibility of clang++'s vector extensions with g++'s vector
// extensions. In comparison to the extensions available in C, the !, ?:, && and
Index: clang/test/Parser/warn-use-alternative-tokens.cpp
===================================================================
--- /dev/null
+++ clang/test/Parser/warn-use-alternative-tokens.cpp
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -Wuse-alternative-tokens-in-expressions -verify %s
+
+void f1()
+{
+ (void)(true && false); // expected-warning{{use 'and' for logical conjunctions}}
+ (void)(true and false); // no warning
+
+ (void)(0 & 1); // expected-warning{{use 'bitand' for bitwise conjunctions}}
+ (void)(0 bitand 1); // no warning
+
+ (void)(0 | 1); // expected-warning{{use 'bitor' for bitwise disjunctions}}
+ (void)(0 bitor 1); // no warning
+
+ (void)(~0); // expected-warning{{use 'compl' for bitwise negation}}
+ (void)(compl 0); // no warning
+
+ (void)(!false); // expected-warning{{use 'not' for logical negation}}
+ (void)(not false); // no warning
+
+ (void)(false || true); // expected-warning{{use 'or' for logical disjunctions}}
+ (void)(false or true); // no warning
+
+ (void)(0 ^ 1); // expected-warning{{use 'xor' for exclusive or}}
+ (void)(0 xor 1); // no warning
+}
+
+template<class T>
+requires true && false // expected-warning{{use 'and' for logical conjunctions}}
+void f2();
+
+template<class T>
+requires true and false // no warning
+void f2();
+
+template<class T>
+requires true || false // expected-warning{{use 'or' for logical disjunctions}}
+void f3();
+
+template<class T>
+requires true or false // no warning
+void f3();
+
+template<class T>
+requires (!true) // expected-warning{{use 'not' for logical negation}}
+void f4();
+
+template<class T>
+requires (not true) // no warning
+void f4();
+
+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); // expected-warning{{use 'and' for logical conjunctions}}
+ (void)(x and y); // no warning
+
+ (void)(x & y); // expected-warning{{use 'bitand' for bitwise conjunctions}}
+ (void)(x bitand y); // no warning
+
+ (void)(x | y); // expected-warning{{use 'bitor' for bitwise disjunctions}}
+ (void)(x bitor y); // no warning
+
+ (void)(~x); // expected-warning{{use 'compl' for bitwise negation}}
+ (void)(compl x); // no warning
+
+ (void)(!x); // expected-warning{{use 'not' for logical negation}}
+ (void)(not x); // no warning
+
+ (void)(x || y); // expected-warning{{use 'or' for logical disjunctions}}
+ (void)(x or y); // no warning
+
+ (void)(x ^ y); // expected-warning{{use 'xor' for exclusive or}}
+ (void)(x xor y); // no warning
+}
Index: clang/test/Parser/warn-use-alternative-tokens.c
===================================================================
--- /dev/null
+++ clang/test/Parser/warn-use-alternative-tokens.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -Wuse-alternative-tokens-in-expressions -verify %s
+
+// expected-no-diagnostics
+
+void f1()
+{
+ (void)(1 && 0);
+ (void)(0 & 1);
+ (void)(0 | 1);
+ (void)(~0);
+ (void)(!0);
+ (void)(0 || 1);
+ (void)(0 ^ 1);
+}
Index: clang/lib/Parse/ParseExpr.cpp
===================================================================
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -317,6 +317,13 @@
if (LHS.isInvalid())
return ExprError();
while (Tok.is(tok::ampamp)) {
+ if (Tok.getSpelling() == "&&") {
+ constexpr int And = 0;
+ constexpr int Logical = 2;
+ constexpr int Conjunctions = 0;
+ Diag(Tok.getLocation(), diag::warn_use_alternative_tokens_in_expressions)
+ << And << Logical << Conjunctions;
+ }
SourceLocation LogicalAndLoc = ConsumeToken();
ExprResult RHS = ParsePrimary();
if (RHS.isInvalid()) {
@@ -351,6 +358,13 @@
if (!LHS.isUsable())
return ExprError();
while (Tok.is(tok::pipepipe)) {
+ if (Tok.getSpelling() == "||") {
+ constexpr int Or = 5;
+ constexpr int Logical = 2;
+ constexpr int Disjunctions = 1;
+ Diag(Tok.getLocation(), diag::warn_use_alternative_tokens_in_expressions)
+ << Or << Logical << Disjunctions;
+ }
SourceLocation LogicalOrLoc = ConsumeToken();
ExprResult RHS =
ParseConstraintLogicalAndExpression(IsTrailingRequiresClause);
@@ -390,6 +404,78 @@
return isFoldOperator(getBinOpPrecedence(Kind, GreaterThanIsOperator, true));
}
+void CheckLogicalOperators(Parser &P, const Token &Tok) {
+ if (Tok.is(tok::ampamp) && Tok.getSpelling() == "&&") {
+ constexpr int And = 0;
+ constexpr int Logical = 2;
+ constexpr int Conjunctions = 0;
+ P.Diag(Tok.getLocation(), diag::warn_use_alternative_tokens_in_expressions)
+ << And << Logical << Conjunctions;
+ return;
+ }
+
+ if (Tok.is(tok::pipepipe) && Tok.getSpelling() == "||") {
+ constexpr int Or = 5;
+ constexpr int Logical = 2;
+ constexpr int Disjunctions = 1;
+ P.Diag(Tok.getLocation(), diag::warn_use_alternative_tokens_in_expressions)
+ << Or << Logical << Disjunctions;
+ return;
+ }
+}
+
+void CheckBitwiseOperators(Parser &P, const Token &Tok) {
+ if (Tok.is(tok::amp) && Tok.getSpelling() == "&") {
+ constexpr int Bitand = 1;
+ constexpr int Bitwise = 1;
+ constexpr int Conjunctions = 0;
+ P.Diag(Tok.getLocation(), diag::warn_use_alternative_tokens_in_expressions)
+ << Bitand << Bitwise << Conjunctions;
+ return;
+ }
+
+ if (Tok.is(tok::pipe) && Tok.getSpelling() == "|") {
+ // TODO: add Sema checking to allow the pipe operator to be usable for
+ // program composition. This will probably need to happen via an attribute
+ // (e.g. [[clang::composable]]).
+ constexpr int Bitor = 2;
+ constexpr int Bitwise = 1;
+ constexpr int Disjunctions = 1;
+ P.Diag(Tok.getLocation(), diag::warn_use_alternative_tokens_in_expressions)
+ << Bitor << Bitwise << Disjunctions;
+ return;
+ }
+
+ if (Tok.is(tok::caret) && Tok.getSpelling() == "^") {
+ constexpr int Xor = 6;
+ constexpr int Bitwise = 0;
+ constexpr int ExclusiveOr = 2;
+ P.Diag(Tok.getLocation(), diag::warn_use_alternative_tokens_in_expressions)
+ << Xor << Bitwise << ExclusiveOr;
+ return;
+ }
+}
+
+void CheckNegationOperators(Parser &P, const Token &Tok) {
+ if (Tok.is(tok::exclaim) && Tok.getSpelling() == "!") {
+ constexpr int Not = 4;
+ constexpr int Logical = 2;
+ constexpr int Negation = 3;
+ P.Diag(Tok.getLocation(), diag::warn_use_alternative_tokens_in_expressions)
+ << Not << Logical << Negation;
+ return;
+ }
+
+ if (Tok.is(tok::tilde) && Tok.getSpelling() == "~") {
+ constexpr int Compl = 3;
+ constexpr int Bitwise = 1;
+ constexpr int Negation = 3;
+ P.Diag(Tok.getLocation(), diag::warn_use_alternative_tokens_in_expressions)
+ << Compl << Bitwise << Negation;
+ return;
+ }
+}
+
/// Parse a binary expression that starts with \p LHS and has a
/// precedence of at least \p MinPrec.
ExprResult
@@ -435,6 +521,11 @@
return LHS;
}
+ if (getLangOpts().CPlusPlus) {
+ CheckLogicalOperators(*this, OpToken);
+ CheckBitwiseOperators(*this, OpToken);
+ }
+
// If the next token is an ellipsis, then this is a fold-expression. Leave
// it alone so we can handle it in the paren expression.
if (isFoldOperator(NextTokPrec) && Tok.is(tok::ellipsis)) {
@@ -1374,6 +1465,10 @@
case tok::kw___imag: { // unary-expression: '__imag' cast-expression [GNU]
if (NotPrimaryExpression)
*NotPrimaryExpression = true;
+
+ if (getLangOpts().CPlusPlus)
+ CheckNegationOperators(*this, Tok);
+
SourceLocation SavedLoc = ConsumeToken();
PreferredType.enterUnary(Actions, Tok.getLocation(), SavedKind, SavedLoc);
Res = ParseCastExpression(AnyCastExpr);
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1547,4 +1547,10 @@
def note_max_tokens_total_override : Note<"total token limit set here">;
+def warn_use_alternative_tokens_in_expressions : Warning<
+ "use '%select{and|bitand|bitor|compl|not|or|xor}0' "
+ "for %select{|bitwise |logical }1"
+ "%select{conjunctions|disjunctions|exclusive or|negation}2">,
+ InGroup<DiagGroup<"use-alternative-tokens-in-expressions">>, DefaultIgnore;
+
} // end of Parser diagnostics
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
