https://github.com/localspook created https://github.com/llvm/llvm-project/pull/175121
This is currently our most expensive check as measured by `--enable-check-profile`, tied with `altera-id-dependent-backward-branch`. I measured using [the usual setup](https://github.com/llvm/llvm-project/pull/174357#issue-3780188615): ```sh hyperfine \ --shell=none \ --prepare='cmake --build build/release --target clang-tidy' \ './build/release/bin/clang-tidy --checks=-*,readability-container-contains all_headers.cpp -header-filter=.* -system-headers -- -std=c++23 -fno-delayed-template-parsing' ``` First, the status quo: ```txt Time (mean ± σ): 5.243 s ± 0.158 s [User: 4.964 s, System: 0.248 s] Range (min … max): 5.133 s … 5.612 s 10 runs ``` This PR improves that in two independent commits. The first commit changes the default traversal mode from `TK_AsIs` to `TK_IgnoreUnlessSpelledInSource`. Result: ```txt Time (mean ± σ): 4.554 s ± 0.100 s [User: 4.273 s, System: 0.248 s] Range (min … max): 4.442 s … 4.785 s 10 runs ``` The second commit merges all 12 of the check's `BinaryOperation` matchers into one, because I found that simply registering a `BinaryOperation` matcher has pretty extreme overhead. Result: ```txt Time (mean ± σ): 3.480 s ± 0.121 s [User: 3.264 s, System: 0.186 s] Range (min … max): 3.400 s … 3.798 s 10 runs ``` >From f5d0a216674c1dbf054eb84c6301297afad4a0d2 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> Date: Thu, 8 Jan 2026 18:36:50 -0800 Subject: [PATCH 1/2] Set TK_IgnoreUnlessSpelledInSource as the default --- .../readability/ContainerContainsCheck.cpp | 82 +++++++++++-------- .../readability/ContainerContainsCheck.h | 2 +- 2 files changed, 47 insertions(+), 37 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp index efcf13d63b4ff..6fe49cfe53a89 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp @@ -47,57 +47,67 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) { const auto StringNpos = anyOf(declRefExpr(to(varDecl(hasName("npos")))), memberExpr(member(hasName("npos")))); - auto AddSimpleMatcher = [&](const auto &Matcher) { - Finder->addMatcher(traverse(TK_IgnoreUnlessSpelledInSource, Matcher), this); - }; - // Find membership tests which use `count()`. - Finder->addMatcher(implicitCastExpr(hasImplicitDestinationType(booleanType()), - hasSourceExpression(CountCall)) - .bind("positiveComparison"), - this); - AddSimpleMatcher( + Finder->addMatcher( + traverse(TK_AsIs, + implicitCastExpr(hasImplicitDestinationType(booleanType()), + hasSourceExpression(CountCall)) + .bind("positiveComparison")), + this); + Finder->addMatcher( binaryOperation(hasOperatorName("!="), hasOperands(CountCall, Literal0)) - .bind("positiveComparison")); - AddSimpleMatcher( + .bind("positiveComparison"), + this); + Finder->addMatcher( binaryOperation(hasLHS(CountCall), hasOperatorName(">"), hasRHS(Literal0)) - .bind("positiveComparison")); - AddSimpleMatcher( + .bind("positiveComparison"), + this); + Finder->addMatcher( binaryOperation(hasLHS(Literal0), hasOperatorName("<"), hasRHS(CountCall)) - .bind("positiveComparison")); - AddSimpleMatcher(binaryOperation(hasLHS(CountCall), hasOperatorName(">="), - hasRHS(Literal1)) - .bind("positiveComparison")); - AddSimpleMatcher(binaryOperation(hasLHS(Literal1), hasOperatorName("<="), - hasRHS(CountCall)) - .bind("positiveComparison")); + .bind("positiveComparison"), + this); + Finder->addMatcher(binaryOperation(hasLHS(CountCall), hasOperatorName(">="), + hasRHS(Literal1)) + .bind("positiveComparison"), + this); + Finder->addMatcher(binaryOperation(hasLHS(Literal1), hasOperatorName("<="), + hasRHS(CountCall)) + .bind("positiveComparison"), + this); // Find inverted membership tests which use `count()`. - AddSimpleMatcher( + Finder->addMatcher( binaryOperation(hasOperatorName("=="), hasOperands(CountCall, Literal0)) - .bind("negativeComparison")); - AddSimpleMatcher(binaryOperation(hasLHS(CountCall), hasOperatorName("<="), - hasRHS(Literal0)) - .bind("negativeComparison")); - AddSimpleMatcher(binaryOperation(hasLHS(Literal0), hasOperatorName(">="), - hasRHS(CountCall)) - .bind("negativeComparison")); - AddSimpleMatcher( + .bind("negativeComparison"), + this); + Finder->addMatcher(binaryOperation(hasLHS(CountCall), hasOperatorName("<="), + hasRHS(Literal0)) + .bind("negativeComparison"), + this); + Finder->addMatcher(binaryOperation(hasLHS(Literal0), hasOperatorName(">="), + hasRHS(CountCall)) + .bind("negativeComparison"), + this); + Finder->addMatcher( binaryOperation(hasLHS(CountCall), hasOperatorName("<"), hasRHS(Literal1)) - .bind("negativeComparison")); - AddSimpleMatcher( + .bind("negativeComparison"), + this); + Finder->addMatcher( binaryOperation(hasLHS(Literal1), hasOperatorName(">"), hasRHS(CountCall)) - .bind("negativeComparison")); + .bind("negativeComparison"), + this); // Find membership tests based on `find() == end()` or `find() == npos`. - AddSimpleMatcher( + Finder->addMatcher( binaryOperation(hasOperatorName("!="), hasOperands(FindCall, anyOf(EndCall, StringNpos))) - .bind("positiveComparison")); - AddSimpleMatcher( + .bind("positiveComparison"), + this); + Finder->addMatcher( binaryOperation(hasOperatorName("=="), hasOperands(FindCall, anyOf(EndCall, StringNpos))) - .bind("negativeComparison")); + .bind("negativeComparison"), + this); } void ContainerContainsCheck::check(const MatchFinder::MatchResult &Result) { diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h index 8e058f20427fd..8d043910fc2ba 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h +++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h @@ -29,7 +29,7 @@ class ContainerContainsCheck : public ClangTidyCheck { return LO.CPlusPlus; } std::optional<TraversalKind> getCheckTraversalKind() const override { - return TK_AsIs; + return TK_IgnoreUnlessSpelledInSource; } }; >From 61bf69ea1158f07b82b2908e8fa4ba8d3315f9ff Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> Date: Thu, 8 Jan 2026 19:28:56 -0800 Subject: [PATCH 2/2] Merge all `BinaryOperation` matchers into one --- .../readability/ContainerContainsCheck.cpp | 74 ++++++------------- 1 file changed, 22 insertions(+), 52 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp index 6fe49cfe53a89..3b4113c4e2ed0 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp @@ -14,6 +14,7 @@ using namespace clang::ast_matchers; namespace clang::tidy::readability { + void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) { const auto Literal0 = integerLiteral(equals(0)); const auto Literal1 = integerLiteral(equals(1)); @@ -47,66 +48,35 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) { const auto StringNpos = anyOf(declRefExpr(to(varDecl(hasName("npos")))), memberExpr(member(hasName("npos")))); - // Find membership tests which use `count()`. Finder->addMatcher( traverse(TK_AsIs, implicitCastExpr(hasImplicitDestinationType(booleanType()), hasSourceExpression(CountCall)) .bind("positiveComparison")), this); - Finder->addMatcher( - binaryOperation(hasOperatorName("!="), hasOperands(CountCall, Literal0)) - .bind("positiveComparison"), - this); - Finder->addMatcher( - binaryOperation(hasLHS(CountCall), hasOperatorName(">"), hasRHS(Literal0)) - .bind("positiveComparison"), - this); - Finder->addMatcher( - binaryOperation(hasLHS(Literal0), hasOperatorName("<"), hasRHS(CountCall)) - .bind("positiveComparison"), - this); - Finder->addMatcher(binaryOperation(hasLHS(CountCall), hasOperatorName(">="), - hasRHS(Literal1)) - .bind("positiveComparison"), - this); - Finder->addMatcher(binaryOperation(hasLHS(Literal1), hasOperatorName("<="), - hasRHS(CountCall)) - .bind("positiveComparison"), - this); - - // Find inverted membership tests which use `count()`. - Finder->addMatcher( - binaryOperation(hasOperatorName("=="), hasOperands(CountCall, Literal0)) - .bind("negativeComparison"), - this); - Finder->addMatcher(binaryOperation(hasLHS(CountCall), hasOperatorName("<="), - hasRHS(Literal0)) - .bind("negativeComparison"), - this); - Finder->addMatcher(binaryOperation(hasLHS(Literal0), hasOperatorName(">="), - hasRHS(CountCall)) - .bind("negativeComparison"), - this); - Finder->addMatcher( - binaryOperation(hasLHS(CountCall), hasOperatorName("<"), hasRHS(Literal1)) - .bind("negativeComparison"), - this); - Finder->addMatcher( - binaryOperation(hasLHS(Literal1), hasOperatorName(">"), hasRHS(CountCall)) - .bind("negativeComparison"), - this); - // Find membership tests based on `find() == end()` or `find() == npos`. - Finder->addMatcher( - binaryOperation(hasOperatorName("!="), - hasOperands(FindCall, anyOf(EndCall, StringNpos))) - .bind("positiveComparison"), - this); + const auto PositiveComparison = + anyOf(allOf(hasOperatorName("!="), hasOperands(CountCall, Literal0)), + allOf(hasLHS(CountCall), hasOperatorName(">"), hasRHS(Literal0)), + allOf(hasLHS(Literal0), hasOperatorName("<"), hasRHS(CountCall)), + allOf(hasLHS(CountCall), hasOperatorName(">="), hasRHS(Literal1)), + allOf(hasLHS(Literal1), hasOperatorName("<="), hasRHS(CountCall)), + allOf(hasOperatorName("!="), + hasOperands(FindCall, anyOf(EndCall, StringNpos)))); + + const auto NegativeComparison = + anyOf(allOf(hasOperatorName("=="), hasOperands(CountCall, Literal0)), + allOf(hasLHS(CountCall), hasOperatorName("<="), hasRHS(Literal0)), + allOf(hasLHS(Literal0), hasOperatorName(">="), hasRHS(CountCall)), + allOf(hasLHS(CountCall), hasOperatorName("<"), hasRHS(Literal1)), + allOf(hasLHS(Literal1), hasOperatorName(">"), hasRHS(CountCall)), + allOf(hasOperatorName("=="), + hasOperands(FindCall, anyOf(EndCall, StringNpos)))); + Finder->addMatcher( - binaryOperation(hasOperatorName("=="), - hasOperands(FindCall, anyOf(EndCall, StringNpos))) - .bind("negativeComparison"), + binaryOperation( + anyOf(allOf(PositiveComparison, expr().bind("positiveComparison")), + allOf(NegativeComparison, expr().bind("negativeComparison")))), this); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
