Use mapAnyOf() and matchers based on it.
Use of binaryOperation() means that modernize-loop-convert and
readability-container-size-empty can now be used with rewritten binary
operators.
Differential Revision: https://reviews.llvm.org/D94131
static internal::Matcher<Stmt>
loopEndingStmt(internal::Matcher<Stmt> Internal) {
- return stmt(anyOf(breakStmt(Internal), returnStmt(Internal),
- gotoStmt(Internal), cxxThrowExpr(Internal),
- callExpr(Internal, callee(functionDecl(isNoReturn())))));
+ return stmt(anyOf(
+ mapAnyOf(breakStmt, returnStmt, gotoStmt, cxxThrowExpr).with(Internal),
+ callExpr(Internal, callee(functionDecl(isNoReturn())))));
}
/// Return whether `Var` was changed in `LoopStmt`.
unless(hasBody(hasDescendant(
loopEndingStmt(forFunction(equalsBoundNode("func")))))));
- Finder->addMatcher(stmt(anyOf(whileStmt(LoopCondition), doStmt(LoopCondition),
- forStmt(LoopCondition)))
+ Finder->addMatcher(mapAnyOf(whileStmt, doStmt, forStmt)
+ .with(LoopCondition)
.bind("loop-stmt"),
this);
}
if (getLangOpts().CPlusPlus) {
// Check for `CON54-CPP`
Finder->addMatcher(
- ifStmt(
- allOf(HasWaitDescendantCpp,
- unless(anyOf(hasDescendant(ifStmt(HasWaitDescendantCpp)),
- hasDescendant(whileStmt(HasWaitDescendantCpp)),
- hasDescendant(forStmt(HasWaitDescendantCpp)),
- hasDescendant(doStmt(HasWaitDescendantCpp)))))
-
- ),
+ ifStmt(HasWaitDescendantCpp,
+ unless(hasDescendant(mapAnyOf(ifStmt, whileStmt, forStmt, doStmt)
+ .with(HasWaitDescendantCpp)))),
this);
} else {
// Check for `CON36-C`
Finder->addMatcher(
- ifStmt(
- allOf(HasWaitDescendantC,
- unless(anyOf(hasDescendant(ifStmt(HasWaitDescendantC)),
- hasDescendant(whileStmt(HasWaitDescendantC)),
- hasDescendant(forStmt(HasWaitDescendantC)),
- hasDescendant(doStmt(HasWaitDescendantC)),
- hasParent(whileStmt()),
- hasParent(compoundStmt(hasParent(whileStmt()))),
- hasParent(forStmt()),
- hasParent(compoundStmt(hasParent(forStmt()))),
- hasParent(doStmt()),
- hasParent(compoundStmt(hasParent(doStmt())))))
-
- ))
-
- ,
+ ifStmt(HasWaitDescendantC,
+ unless(anyOf(
+ hasDescendant(mapAnyOf(ifStmt, whileStmt, forStmt, doStmt)
+ .with(HasWaitDescendantC)),
+ hasParent(mapAnyOf(whileStmt, forStmt, doStmt)),
+ hasParent(compoundStmt(
+ hasParent(mapAnyOf(whileStmt, forStmt, doStmt))))))),
this);
}
}
// Detect suspicious calls to string compare:
// 'if (strcmp())' -> 'if (strcmp() != 0)'
Finder->addMatcher(
- stmt(anyOf(ifStmt(hasCondition(StringCompareCallExpr)),
- whileStmt(hasCondition(StringCompareCallExpr)),
- doStmt(hasCondition(StringCompareCallExpr)),
- forStmt(hasCondition(StringCompareCallExpr)),
+ stmt(anyOf(mapAnyOf(ifStmt, whileStmt, doStmt, forStmt)
+ .with(hasCondition(StringCompareCallExpr)),
binaryOperator(hasAnyOperatorName("&&", "||"),
hasEitherOperand(StringCompareCallExpr))))
.bind("missing-comparison"),
// Self-check: Code compares something with 'this' pointer. We don't check
// whether it is actually the parameter what we compare.
- const auto HasNoSelfCheck = cxxMethodDecl(unless(anyOf(
- hasDescendant(binaryOperator(hasAnyOperatorName("==", "!="),
- has(ignoringParenCasts(cxxThisExpr())))),
- hasDescendant(cxxOperatorCallExpr(
- hasAnyOverloadedOperatorName("==", "!="), argumentCountIs(2),
- has(ignoringParenCasts(cxxThisExpr())))))));
+ const auto HasNoSelfCheck = cxxMethodDecl(unless(hasDescendant(
+ binaryOperation(hasAnyOperatorName("==", "!="),
+ hasEitherOperand(ignoringParenCasts(cxxThisExpr()))))));
// Both copy-and-swap and copy-and-move method creates a copy first and
// assign it to 'this' with swap or move.
// Assignment. In addition to the overloaded assignment operator,
// test for built-in assignment as well, since template functions
// may be instantiated to use std::move() on built-in types.
- binaryOperator(hasOperatorName("="), hasLHS(DeclRefMatcher)),
- cxxOperatorCallExpr(hasOverloadedOperatorName("="),
- hasArgument(0, DeclRefMatcher)),
+ binaryOperation(hasOperatorName("="), hasLHS(DeclRefMatcher)),
// Declaration. We treat this as a type of reinitialization too,
// so we don't need to treat it separately.
declStmt(hasDescendant(equalsNode(MovedVariable))),
MemberExprOrSourceObject);
const auto IsSourceMutatingAssignment = traverse(
- TK_AsIs,
- expr(anyOf(binaryOperator(isAssignmentOperator(), hasLHS(IsPartOfSource))
- .bind(MutatingOperatorName),
- cxxOperatorCallExpr(isAssignmentOperator(),
- hasArgument(0, IsPartOfSource))
- .bind(MutatingOperatorName))));
+ TK_AsIs, binaryOperation(hasOperatorName("="), hasLHS(IsPartOfSource))
+ .bind(MutatingOperatorName));
const auto MemberExprOrSelf = anyOf(memberExpr(), cxxThisExpr());
unless(hasDescendant(expr(unless(MemberExprOrSelf)))), MemberExprOrSelf);
const auto IsSelfMutatingAssignment =
- expr(anyOf(binaryOperator(isAssignmentOperator(), hasLHS(IsPartOfSelf)),
- cxxOperatorCallExpr(isAssignmentOperator(),
- hasArgument(0, IsPartOfSelf))));
+ binaryOperation(isAssignmentOperator(), hasLHS(IsPartOfSelf));
const auto IsSelfMutatingMemberFunction =
functionDecl(hasBody(hasDescendant(IsSelfMutatingAssignment)));
// Check if the 'goto' is used for control flow other than jumping
// out of a nested loop.
- auto Loop = stmt(anyOf(forStmt(), cxxForRangeStmt(), whileStmt(), doStmt()));
- auto NestedLoop =
- stmt(anyOf(forStmt(hasAncestor(Loop)), cxxForRangeStmt(hasAncestor(Loop)),
- whileStmt(hasAncestor(Loop)), doStmt(hasAncestor(Loop))));
+ auto Loop = mapAnyOf(forStmt, cxxForRangeStmt, whileStmt, doStmt);
+ auto NestedLoop = Loop.with(hasAncestor(Loop));
Finder->addMatcher(gotoStmt(anyOf(unless(hasAncestor(NestedLoop)),
unless(isForwardJumping())))
allOf(callee(namedDecl(hasAnyName("isa", "cast", "cast_or_null",
"dyn_cast", "dyn_cast_or_null"))
.bind("func")),
- hasArgument(0, anyOf(declRefExpr().bind("arg"),
- cxxMemberCallExpr().bind("arg"))))))
+ hasArgument(
+ 0,
+ mapAnyOf(declRefExpr, cxxMemberCallExpr).bind("arg")))))
.bind("rhs");
Finder->addMatcher(
StatementMatcher IteratorComparisonMatcher = expr(
ignoringParenImpCasts(declRefExpr(to(varDecl().bind(ConditionVarName)))));
- auto OverloadedNEQMatcher = ignoringImplicit(
- cxxOperatorCallExpr(hasOverloadedOperatorName("!="), argumentCountIs(2),
- hasArgument(0, IteratorComparisonMatcher),
- hasArgument(1, IteratorBoundMatcher)));
-
// This matcher tests that a declaration is a CXXRecordDecl that has an
// overloaded operator*(). If the operator*() returns by value instead of by
// reference then the return type is tagged with DerefByValueResultName.
containsDeclaration(0, InitDeclMatcher),
containsDeclaration(1, EndDeclMatcher)),
declStmt(hasSingleDecl(InitDeclMatcher)))),
- hasCondition(
- anyOf(binaryOperator(hasOperatorName("!="),
- hasLHS(IteratorComparisonMatcher),
- hasRHS(IteratorBoundMatcher)),
- binaryOperator(hasOperatorName("!="),
- hasLHS(IteratorBoundMatcher),
- hasRHS(IteratorComparisonMatcher)),
- OverloadedNEQMatcher)),
+ hasCondition(ignoringImplicit(binaryOperation(
+ hasOperatorName("!="), hasOperands(IteratorComparisonMatcher,
+ IteratorBoundMatcher)))),
hasIncrement(anyOf(
unaryOperator(hasOperatorName("++"),
hasUnaryOperand(declRefExpr(
auto LHS = memberExpr(hasObjectExpression(cxxThisExpr()),
member(fieldDecl(equalsNode(Field))));
auto RHS = accessToFieldInVar(Field, Param);
- if (match(
- traverse(TK_AsIs,
- compoundStmt(has(ignoringParenImpCasts(stmt(anyOf(
- binaryOperator(hasOperatorName("="), hasLHS(LHS),
- hasRHS(RHS)),
- cxxOperatorCallExpr(
- hasOverloadedOperatorName("="), argumentCountIs(2),
- hasArgument(0, LHS), hasArgument(1, RHS)))))))),
- *Compound, *Context)
+ if (match(traverse(TK_AsIs,
+ compoundStmt(has(ignoringParenImpCasts(binaryOperation(
+ hasOperatorName("="), hasLHS(LHS), hasRHS(RHS)))))),
+ *Compound, *Context)
.empty())
return false;
}
Finder->addMatcher(MoveCallMatcher, this);
- auto ConstParamMatcher = forEachArgumentWithParam(
- MoveCallMatcher, parmVarDecl(hasType(references(isConstQualified()))));
-
- Finder->addMatcher(callExpr(ConstParamMatcher).bind("receiving-expr"), this);
Finder->addMatcher(
- traverse(TK_AsIs,
- cxxConstructExpr(ConstParamMatcher).bind("receiving-expr")),
+ invocation(forEachArgumentWithParam(
+ MoveCallMatcher,
+ parmVarDecl(hasType(references(isConstQualified())))))
+ .bind("receiving-expr"),
this);
}
const char *ExprName = "__booleanContextExpr";
auto Result =
expr(expr().bind(ExprName),
- anyOf(hasParent(varDecl(hasType(booleanType()))),
+ anyOf(hasParent(
+ mapAnyOf(varDecl, fieldDecl).with(hasType(booleanType()))),
hasParent(cxxConstructorDecl(
hasAnyConstructorInitializer(cxxCtorInitializer(
withInitializer(expr(equalsBoundNode(ExprName))),
forField(hasType(booleanType())))))),
- hasParent(fieldDecl(hasType(booleanType()))),
hasParent(stmt(anyOf(
explicitCastExpr(hasDestinationType(booleanType())),
- ifStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
- doStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
- whileStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
- forStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
- conditionalOperator(
- hasCondition(expr(equalsBoundNode(ExprName)))),
+ mapAnyOf(ifStmt, doStmt, whileStmt, forStmt,
+ conditionalOperator)
+ .with(hasCondition(expr(equalsBoundNode(ExprName)))),
parenListExpr(hasParent(varDecl(hasType(booleanType())))),
parenExpr(hasParent(
explicitCastExpr(hasDestinationType(booleanType())))),
returnStmt(forFunction(returns(booleanType()))),
cxxUnresolvedConstructExpr(hasType(booleanType())),
- callExpr(hasAnyArgumentWithParam(
+ invocation(hasAnyArgumentWithParam(
expr(equalsBoundNode(ExprName)),
parmVarDecl(hasType(booleanType())))),
binaryOperator(hasAnyOperatorName("&&", "||")),
expr(hasType(pointsTo(ValidContainer))).bind("Pointee"))),
expr(hasType(ValidContainer)).bind("STLObject"));
Finder->addMatcher(
- cxxOperatorCallExpr(
- unless(isInTemplateInstantiation()),
- hasAnyOverloadedOperatorName("==", "!="),
- anyOf(allOf(hasArgument(0, WrongComparend), hasArgument(1, STLArg)),
- allOf(hasArgument(0, STLArg), hasArgument(1, WrongComparend))),
- unless(hasAncestor(
- cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
- .bind("BinCmp"),
- this);
- Finder->addMatcher(
- binaryOperator(hasAnyOperatorName("==", "!="),
- anyOf(allOf(hasLHS(WrongComparend), hasRHS(STLArg)),
- allOf(hasLHS(STLArg), hasRHS(WrongComparend))),
- unless(hasAncestor(
- cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
+ binaryOperation(unless(isInTemplateInstantiation()),
+ hasAnyOperatorName("==", "!="),
+ hasOperands(ignoringParenImpCasts(WrongComparend),
+ ignoringParenImpCasts(STLArg)),
+ unless(hasAncestor(cxxMethodDecl(
+ ofClass(equalsBoundNode("container"))))))
.bind("BinCmp"),
this);
}
Result.Nodes.getNodeAs<Expr>("MemberCallObject");
const auto *BinCmp = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("BinCmp");
const auto *BinCmpTempl = Result.Nodes.getNodeAs<BinaryOperator>("BinCmp");
+ const auto *BinCmpRewritten =
+ Result.Nodes.getNodeAs<CXXRewrittenBinaryOperator>("BinCmp");
const auto *BinaryOp = Result.Nodes.getNodeAs<BinaryOperator>("SizeBinaryOp");
const auto *Pointee = Result.Nodes.getNodeAs<Expr>("Pointee");
const auto *E =
}
Hint = FixItHint::CreateReplacement(BinCmpTempl->getSourceRange(),
ReplacementText);
+ } else if (BinCmpRewritten) {
+ if (BinCmpRewritten->getOpcode() == BinaryOperatorKind::BO_NE) {
+ ReplacementText = "!" + ReplacementText;
+ }
+ Hint = FixItHint::CreateReplacement(BinCmpRewritten->getSourceRange(),
+ ReplacementText);
} else if (BinaryOp) { // Determine the correct transformation.
bool Negation = false;
const bool ContainerIsLHS =
"for emptiness instead of 'size'")
<< Hint;
} else {
- WarnLoc = BinCmpTempl ? BinCmpTempl->getBeginLoc()
- : (BinCmp ? BinCmp->getBeginLoc() : SourceLocation{});
+ WarnLoc = BinCmpTempl
+ ? BinCmpTempl->getBeginLoc()
+ : (BinCmp ? BinCmp->getBeginLoc()
+ : (BinCmpRewritten ? BinCmpRewritten->getBeginLoc()
+ : SourceLocation{}));
diag(WarnLoc, "the 'empty' method should be used to check "
"for emptiness instead of comparing to an empty object")
<< Hint;
has(compoundStmt(hasAnySubstatement(returnStmt(unless(has(expr())))))
.bind("return"))),
this);
- auto CompoundContinue =
- has(compoundStmt(hasAnySubstatement(continueStmt())).bind("continue"));
Finder->addMatcher(
- stmt(anyOf(forStmt(), cxxForRangeStmt(), whileStmt(), doStmt()),
- CompoundContinue),
+ mapAnyOf(forStmt, cxxForRangeStmt, whileStmt, doStmt)
+ .with(hasBody(compoundStmt(hasAnySubstatement(continueStmt()))
+ .bind("continue"))),
this);
}
substTemplateTypeParmType(hasReplacementType(ConstReferenceOrValue))));
auto UsedAsConstRefOrValueArg = forEachArgumentWithParam(
DeclRefToVar, parmVarDecl(hasType(ConstReferenceOrValueOrReplaced)));
- Matches = match(findAll(callExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
- extractNodesByIdTo(Matches, "declRef", DeclRefs);
- Matches =
- match(findAll(cxxConstructExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
+ Matches = match(findAll(invocation(UsedAsConstRefOrValueArg)), Stmt, Context);
extractNodesByIdTo(Matches, "declRef", DeclRefs);
// References and pointers to const assignments.
Matches =
--- /dev/null
+// RUN: %check_clang_tidy -std=c++20 %s modernize-loop-convert %t -- -- -I %S/Inputs/modernize-loop-convert
+
+namespace std {
+struct strong_ordering {
+ int n;
+ constexpr operator int() const { return n; }
+ static const strong_ordering equal, greater, less;
+};
+constexpr strong_ordering strong_ordering::equal = {0};
+constexpr strong_ordering strong_ordering::greater = {1};
+constexpr strong_ordering strong_ordering::less = {-1};
+} // namespace std
+
+struct HasSpaceshipMem {
+ typedef int value_type;
+
+ struct iterator {
+ value_type &operator*();
+ const value_type &operator*() const;
+ iterator &operator++();
+ void insert(value_type);
+ value_type X;
+ constexpr auto operator<=>(const HasSpaceshipMem::iterator &) const = default;
+ };
+
+ iterator begin();
+ iterator end();
+};
+
+struct OpEqOnly {
+ typedef int value_type;
+ struct iterator {
+ value_type &operator*();
+ const value_type &operator*() const;
+ iterator &operator++();
+ bool operator==(const iterator &other) const;
+ void insert(value_type);
+ value_type X;
+ };
+ iterator begin();
+ iterator end();
+};
+
+void rewritten() {
+ OpEqOnly Oeo;
+ for (OpEqOnly::iterator It = Oeo.begin(), E = Oeo.end(); It != E; ++It) {
+ (void)*It;
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (int & It : Oeo)
+ // CHECK-FIXES-NEXT: (void)It;
+
+ HasSpaceshipMem Hsm;
+ for (HasSpaceshipMem::iterator It = Hsm.begin(), E = Hsm.end(); It != E; ++It) {
+ (void)*It;
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (int & It : Hsm)
+ // CHECK-FIXES-NEXT: (void)It;
+}
--- /dev/null
+// RUN: %check_clang_tidy -std=c++20 %s readability-container-size-empty %t -- -- -fno-delayed-template-parsing
+
+namespace std {
+struct strong_ordering {
+ int n;
+ constexpr operator int() const { return n; }
+ static const strong_ordering equal, greater, less;
+};
+constexpr strong_ordering strong_ordering::equal = {0};
+constexpr strong_ordering strong_ordering::greater = {1};
+constexpr strong_ordering strong_ordering::less = {-1};
+} // namespace std
+
+template <typename T>
+struct OpEqOnly {
+ OpEqOnly();
+ bool operator==(const OpEqOnly<T> &other) const;
+ unsigned long size() const;
+ bool empty() const;
+};
+
+template <typename T>
+struct HasSpaceshipMem {
+ HasSpaceshipMem();
+ bool operator<=>(const HasSpaceshipMem<T> &other) const = default;
+ unsigned long size() const;
+ bool empty() const;
+};
+
+void returnsVoid() {
+ OpEqOnly<int> OEO;
+ HasSpaceshipMem<int> HSM;
+
+ if (OEO != OpEqOnly<int>())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness
+ // CHECK-FIXES: {{^ }}if (!OEO.empty()){{$}}
+ // CHECK-MESSAGES: :19:8: note: method 'OpEqOnly'::empty() defined here
+ if (HSM != HasSpaceshipMem<int>())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness
+ // CHECK-FIXES: {{^ }}if (!HSM.empty()){{$}}
+ // CHECK-MESSAGES: :27:8: note: method 'HasSpaceshipMem'::empty() defined here
+}