https://github.com/localspook updated https://github.com/llvm/llvm-project/pull/187069
>From 9ee95a306065c4a135cca0b1d9c4af15e1b699ff Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> Date: Tue, 17 Mar 2026 17:09:27 +0000 Subject: [PATCH 1/2] [clang-tidy] Extend `performrformance-faster-string-find` to handle ternary operator arguments --- .../performance/FasterStringFindCheck.cpp | 57 ++++++++++++------- clang-tools-extra/docs/ReleaseNotes.rst | 3 + .../checks/performance/faster-string-find.rst | 2 + .../performance/faster-string-find.cpp | 16 ++++++ 4 files changed, 59 insertions(+), 19 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp b/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp index 1d9325166e341..43c0bd37dab9a 100644 --- a/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp @@ -11,28 +11,24 @@ #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "llvm/Support/raw_ostream.h" -#include <optional> using namespace clang::ast_matchers; namespace clang::tidy::performance { -static std::optional<std::string> -makeCharacterLiteral(const StringLiteral *Literal) { +static std::string makeCharacterLiteral(const StringLiteral *Literal) { std::string Result; { llvm::raw_string_ostream OS(Result); Literal->outputString(OS); } // Now replace the " with '. - auto OpenPos = Result.find_first_of('"'); - if (OpenPos == std::string::npos) - return std::nullopt; + const size_t OpenPos = Result.find_first_of('"'); + assert(OpenPos != std::string::npos); Result[OpenPos] = '\''; - auto ClosePos = Result.find_last_of('"'); - if (ClosePos == std::string::npos) - return std::nullopt; + const size_t ClosePos = Result.find_last_of('"'); + assert(ClosePos != std::string::npos); Result[ClosePos] = '\''; // "'" is OK, but ''' is not, so add a backslash @@ -42,6 +38,23 @@ makeCharacterLiteral(const StringLiteral *Literal) { return Result; } +template <typename T> +static bool forAllLeavesOfTernaryTree(const Expr *E, const T &HandleNode) { + if constexpr (std::is_void_v<decltype(HandleNode(E))>) { + return forAllLeavesOfTernaryTree(E, [&](const Expr *Exp) { + HandleNode(Exp); + return true; + }); + } else { + E = E->IgnoreParenImpCasts(); + if (const auto *Ternary = dyn_cast<ConditionalOperator>(E)) { + return forAllLeavesOfTernaryTree(Ternary->getTrueExpr(), HandleNode) && + forAllLeavesOfTernaryTree(Ternary->getFalseExpr(), HandleNode); + } + return HandleNode(E); + } +} + FasterStringFindCheck::FasterStringFindCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), @@ -55,8 +68,7 @@ void FasterStringFindCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { } void FasterStringFindCheck::registerMatchers(MatchFinder *Finder) { - const auto SingleChar = - ignoringParenCasts(stringLiteral(hasSize(1)).bind("literal")); + const auto SingleChar = expr().bind("literal"); const auto StringExpr = expr(hasType(hasUnqualifiedDesugaredType( recordType(hasDeclaration(recordDecl(hasAnyName(StringLikeClasses))))))); @@ -79,18 +91,25 @@ void FasterStringFindCheck::registerMatchers(MatchFinder *Finder) { } void FasterStringFindCheck::check(const MatchFinder::MatchResult &Result) { - const auto *Literal = Result.Nodes.getNodeAs<StringLiteral>("literal"); const auto *FindFunc = Result.Nodes.getNodeAs<FunctionDecl>("func"); + const auto *Literal = Result.Nodes.getNodeAs<Expr>("literal"); - auto Replacement = makeCharacterLiteral(Literal); - if (!Replacement) + if (!forAllLeavesOfTernaryTree(Literal, [](const Expr *E) { + const auto *Literal = dyn_cast<StringLiteral>(E); + return Literal && Literal->getLength() == 1; + })) return; - diag(Literal->getBeginLoc(), "%0 called with a string literal consisting of " - "a single character; consider using the more " - "effective overload accepting a character") - << FindFunc - << FixItHint::CreateReplacement(Literal->getSourceRange(), *Replacement); + const auto Diag = diag(Literal->getBeginLoc(), + "%0 called with a string literal consisting of " + "a single character; consider using the more " + "effective overload accepting a character") + << FindFunc; + + forAllLeavesOfTernaryTree(Literal, [&](const Expr *E) { + Diag << FixItHint::CreateReplacement( + E->getSourceRange(), makeCharacterLiteral(cast<StringLiteral>(E))); + }); } } // namespace clang::tidy::performance diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index c9a170a9e8660..61ef1585d6f2f 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -319,6 +319,9 @@ Changes in existing checks - Now analyzes calls to the ``starts_with``, ``ends_with``, ``contains``, and ``operator+=`` string member functions. + - Now analyzes cases where the argument to a string function is a ternary + operator, all the branches of which are single-character strings. + - Fixes false negatives when using ``std::set`` from ``libstdc++``. - Improved :doc:`performance-inefficient-string-concatenation diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/faster-string-find.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/faster-string-find.rst index e7ed869acc8ad..4cfc9fa0b651d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/performance/faster-string-find.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/performance/faster-string-find.rst @@ -12,10 +12,12 @@ Examples: .. code-block:: c++ str.find("A"); + str.contains(foo ? "0" : "1"); // becomes str.find('A'); + str.contains(foo ? '0' : '1'); Options ------- diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/faster-string-find.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/faster-string-find.cpp index 3999049a10718..5bc490b4ab0c3 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance/faster-string-find.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/faster-string-find.cpp @@ -46,6 +46,22 @@ void StringFind() { // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'find' called with a string literal // CHECK-FIXES: Str.find('\''); + // Works with arbitrarily complex ternary operators. + Str.find(true ? "a" : "b"); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'find' called with a string literal consisting of a single character; consider using the more effective overload accepting a character [performance-faster-string-find] + // CHECK-FIXES: Str.find(true ? 'a' : 'b'); + + Str.find(true ? (false ? "a" : "b") : "c"); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'find' called with a string literal consisting of a single character; consider using the more effective overload accepting a character [performance-faster-string-find] + // CHECK-FIXES: Str.find(true ? (false ? 'a' : 'b') : 'c'); + + Str.find(true ? "a" : true ? "b" : true ? "c" : "d"); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'find' called with a string literal consisting of a single character; consider using the more effective overload accepting a character [performance-faster-string-find] + // CHECK-FIXES: Str.find(true ? 'a' : true ? 'b' : true ? 'c' : 'd'); + + Str.find(true ? "a" : true ? "b" : true ? "c" : "not one character"); + Str.find("x" ?: "y"); + // Other methods that can also be replaced Str.rfind("a"); // CHECK-MESSAGES: [[@LINE-1]]:13: warning: 'rfind' called with a string literal >From 72be3e1b0f1f6c51a683217463a05eadb24278e4 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> Date: Fri, 20 Mar 2026 09:38:04 -0500 Subject: [PATCH 2/2] NFC: simplify code, rename some variables --- .../performance/FasterStringFindCheck.cpp | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp b/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp index 43c0bd37dab9a..9858c32d7fe4f 100644 --- a/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp @@ -39,20 +39,13 @@ static std::string makeCharacterLiteral(const StringLiteral *Literal) { } template <typename T> -static bool forAllLeavesOfTernaryTree(const Expr *E, const T &HandleNode) { - if constexpr (std::is_void_v<decltype(HandleNode(E))>) { - return forAllLeavesOfTernaryTree(E, [&](const Expr *Exp) { - HandleNode(Exp); - return true; - }); - } else { - E = E->IgnoreParenImpCasts(); - if (const auto *Ternary = dyn_cast<ConditionalOperator>(E)) { - return forAllLeavesOfTernaryTree(Ternary->getTrueExpr(), HandleNode) && - forAllLeavesOfTernaryTree(Ternary->getFalseExpr(), HandleNode); - } - return HandleNode(E); +static bool forAllLeavesOfTernaryTree(const Expr *Node, const T &HandleLeaf) { + Node = Node->IgnoreParenImpCasts(); + if (const auto *Ternary = dyn_cast<ConditionalOperator>(Node)) { + return forAllLeavesOfTernaryTree(Ternary->getTrueExpr(), HandleLeaf) && + forAllLeavesOfTernaryTree(Ternary->getFalseExpr(), HandleLeaf); } + return HandleLeaf(Node); } FasterStringFindCheck::FasterStringFindCheck(StringRef Name, @@ -109,6 +102,7 @@ void FasterStringFindCheck::check(const MatchFinder::MatchResult &Result) { forAllLeavesOfTernaryTree(Literal, [&](const Expr *E) { Diag << FixItHint::CreateReplacement( E->getSourceRange(), makeCharacterLiteral(cast<StringLiteral>(E))); + return true; }); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
