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/4] [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/4] 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; }); } >From 6172d9ed0a017cb57867f839bf874102b6f2c112 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> Date: Fri, 20 Mar 2026 09:40:37 -0500 Subject: [PATCH 3/4] Update warning message --- .../clang-tidy/performance/FasterStringFindCheck.cpp | 2 +- .../clang-tidy/checkers/performance/faster-string-find.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp b/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp index 9858c32d7fe4f..b186a3c26f31f 100644 --- a/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp @@ -96,7 +96,7 @@ void FasterStringFindCheck::check(const MatchFinder::MatchResult &Result) { 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") + "efficient overload accepting a character") << FindFunc; forAllLeavesOfTernaryTree(Literal, [&](const Expr *E) { 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 5bc490b4ab0c3..0ddda89ca4337 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 @@ -48,15 +48,15 @@ void StringFind() { // 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-MESSAGES: [[@LINE-1]]:12: warning: 'find' called with a string literal // 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-MESSAGES: [[@LINE-1]]:12: warning: 'find' called with a string literal // 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-MESSAGES: [[@LINE-1]]:12: warning: 'find' called with a string literal // CHECK-FIXES: Str.find(true ? 'a' : true ? 'b' : true ? 'c' : 'd'); Str.find(true ? "a" : true ? "b" : true ? "c" : "not one character"); >From e8381578710bdedf119de0cf66ea5d7ef3d02267 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> Date: Sun, 22 Mar 2026 11:31:56 -0700 Subject: [PATCH 4/4] More tests; move function into utils/ --- .../performance/FasterStringFindCheck.cpp | 15 +++----------- clang-tools-extra/clang-tidy/utils/ASTUtils.h | 17 ++++++++++++++++ .../performance/faster-string-find.cpp | 20 +++++++++++++++++++ 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp b/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp index b186a3c26f31f..4774f07bd75e7 100644 --- a/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "FasterStringFindCheck.h" +#include "../utils/ASTUtils.h" #include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" @@ -38,16 +39,6 @@ static std::string makeCharacterLiteral(const StringLiteral *Literal) { return Result; } -template <typename T> -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, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), @@ -87,7 +78,7 @@ void FasterStringFindCheck::check(const MatchFinder::MatchResult &Result) { const auto *FindFunc = Result.Nodes.getNodeAs<FunctionDecl>("func"); const auto *Literal = Result.Nodes.getNodeAs<Expr>("literal"); - if (!forAllLeavesOfTernaryTree(Literal, [](const Expr *E) { + if (!utils::forAllLeavesOfTernaryTree(Literal, [](const Expr *E) { const auto *Literal = dyn_cast<StringLiteral>(E); return Literal && Literal->getLength() == 1; })) @@ -99,7 +90,7 @@ void FasterStringFindCheck::check(const MatchFinder::MatchResult &Result) { "efficient overload accepting a character") << FindFunc; - forAllLeavesOfTernaryTree(Literal, [&](const Expr *E) { + utils::forAllLeavesOfTernaryTree(Literal, [&](const Expr *E) { Diag << FixItHint::CreateReplacement( E->getSourceRange(), makeCharacterLiteral(cast<StringLiteral>(E))); return true; diff --git a/clang-tools-extra/clang-tidy/utils/ASTUtils.h b/clang-tools-extra/clang-tidy/utils/ASTUtils.h index 808cd4a54fd1e..589461b1e1919 100644 --- a/clang-tools-extra/clang-tidy/utils/ASTUtils.h +++ b/clang-tools-extra/clang-tidy/utils/ASTUtils.h @@ -45,6 +45,23 @@ bool areStatementsIdentical(const Stmt *FirstStmt, const Stmt *SecondStmt, const IndirectFieldDecl * findOutermostIndirectFieldDeclForField(const FieldDecl *FD); +// Given an expression containing arbitrarily nested ternary operators, such as: +// foo ? 10 : (bar ? rand() : 0) +// Executes the given callback on each of the leaf nodes (in this case, +// on '10', 'rand()', and '0'). +// +// If the callback ever returns false, traversal is aborted and +// 'forAllLeavesOfTernaryTree' returns false too. +template <typename T> +inline 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); +} + } // namespace clang::tidy::utils #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_ASTUTILS_H 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 e4bf05f4631e2..29ec8bf0d949b 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 @@ -26,6 +26,18 @@ void StringFind() { // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'find' called with a string literal consisting of a single character; consider using the more efficient overload accepting a character [performance-faster-string-find] // CHECK-FIXES: Str.find('a'); + Str.find("x" + + ""); + // CHECK-MESSAGES: [[@LINE-3]]:12: warning: 'find' called with a string literal consisting of a single character; consider using the more efficient overload accepting a character [performance-faster-string-find] + // CHECK-FIXES: Str.find('x'); + + Str.find("" + + "k"); + // CHECK-MESSAGES: [[@LINE-3]]:12: warning: 'find' called with a string literal consisting of a single character; consider using the more efficient overload accepting a character [performance-faster-string-find] + // CHECK-FIXES: Str.find('k'); + // Works with the pos argument. Str.find("a", 1); // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'find' called with a string literal @@ -155,8 +167,16 @@ int FindStr() { #define STR_MACRO(str) str.find("A") #define POS_MACRO(pos) std::string().find("A",pos) +#define ONE_CHAR_STRING "m" int Macros() { + std::string s; + + // FIXME: this is a false positive. + s.find(ONE_CHAR_STRING); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'find' called with a string literal + // CHECK-FIXES: s.find('m'); + return STR_MACRO(std::string()) + POS_MACRO(1); // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'find' called with a string literal // CHECK-MESSAGES: [[@LINE-2]]:37: warning: 'find' called with a string literal _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
