https://github.com/localspook updated https://github.com/llvm/llvm-project/pull/182783
>From 4b7c37883651d1ebe47f8f07454ae53a2b4280f3 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> Date: Sun, 22 Feb 2026 15:40:05 -0800 Subject: [PATCH] [clang-tidy] Simplify and generalize `performance-string-view-conversions` --- .../StringViewConversionsCheck.cpp | 78 ++++++------------- .../performance/string-view-conversions.cpp | 48 ++++++++++++ 2 files changed, 73 insertions(+), 53 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/StringViewConversionsCheck.cpp b/clang-tools-extra/clang-tidy/performance/StringViewConversionsCheck.cpp index 8cbc1c1b596ae..65b65e81cb211 100644 --- a/clang-tools-extra/clang-tidy/performance/StringViewConversionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/StringViewConversionsCheck.cpp @@ -18,8 +18,6 @@ static auto getStringTypeMatcher(StringRef CharType) { return hasCanonicalType(hasDeclaration(cxxRecordDecl(hasName(CharType)))); } -static auto isCStrOrData() { return hasAnyName("c_str", "data"); } - void StringViewConversionsCheck::registerMatchers(MatchFinder *Finder) { // Matchers for std::basic_[w|u8|u16|u32]string[_view] families. const auto IsStdString = getStringTypeMatcher("::std::basic_string"); @@ -49,56 +47,38 @@ void StringViewConversionsCheck::registerMatchers(MatchFinder *Finder) { const auto RedundantFunctionalCast = cxxFunctionalCastExpr( hasType(IsStdString), hasDescendant(RedundantStringConstruction)); - // Match method calls on std::string that modify or use the string, such as - // operator+, append(), substr(), etc. - // Exclude c_str()/data() as they are handled later. - const auto HasStringOperatorCall = hasDescendant(cxxOperatorCallExpr( - hasOverloadedOperatorName("+"), hasType(IsStdString))); - const auto HasStringMethodCall = hasDescendant(cxxMemberCallExpr( - on(hasType(IsStdString)), unless(callee(cxxMethodDecl(isCStrOrData()))))); - - const auto IsCallReturningString = callExpr(hasType(IsStdString)); - const auto IsImplicitStringViewFromCall = - cxxConstructExpr(hasType(IsStdStringView), - hasArgument(0, ignoringImplicit(IsCallReturningString))); + const auto RedundantTemporaryString = + expr(anyOf(RedundantStringConstruction, RedundantFunctionalCast)); // Matches std::string(...).[c_str()|.data()] - const auto RedundantStringWithCStr = cxxMemberCallExpr( - callee(cxxMethodDecl(isCStrOrData())), - on(ignoringParenImpCasts( - anyOf(RedundantStringConstruction, RedundantFunctionalCast)))); - - // Main matcher: finds function calls where: - // 1. A parameter has type string_view - // 2. The corresponding argument contains a redundant std::string construction - // (either functional cast syntax or direct construction/brace init) - // 3. The argument does NOT involve: - // - String concatenation with operator+ (string_view doesn't support it) - // - Method calls on the std::string (like append(), substr(), etc.) + const auto RedundantStringWithCStr = + cxxMemberCallExpr(callee(cxxMethodDecl(hasAnyName("c_str", "data"))), + on(ignoringParenImpCasts(RedundantTemporaryString))); + + // Main matcher: finds cases where an expression that's convertible to a + // std::string_view, instead of being used to construct the std::string_view + // directly, is first needlessly converted to a std::string. + Finder->addMatcher( + cxxMemberCallExpr( + callee(memberExpr(member(hasName("operator basic_string_view")), + has(ignoringImplicit(RedundantTemporaryString.bind( + "redundantExpr")))))) + .bind("stringView"), + this); + Finder->addMatcher( - callExpr(forEachArgumentWithParam( - expr(hasType(IsStdStringView), - // Ignore cases where the argument is a function call - unless(ignoringParenImpCasts(IsImplicitStringViewFromCall)), - // Match either syntax for std::string construction or - // .c_str()/.data() pattern - hasDescendant(expr(anyOf(RedundantFunctionalCast, - RedundantStringConstruction, - RedundantStringWithCStr)) - .bind("redundantExpr")), - // Exclude cases of std::string methods or operator+ calls - // (but allow c_str/data since we handle them) - unless(anyOf(HasStringOperatorCall, HasStringMethodCall))) - .bind("paramExpr"), - parmVarDecl(hasType(IsStdStringView)))), + cxxConstructExpr( + argumentCountIs(1), + hasArgument(0, RedundantStringWithCStr.bind("redundantExpr"))) + .bind("stringView"), this); } void StringViewConversionsCheck::check(const MatchFinder::MatchResult &Result) { - const auto *ParamExpr = Result.Nodes.getNodeAs<Expr>("paramExpr"); + const auto *StringView = Result.Nodes.getNodeAs<Expr>("stringView"); const auto *RedundantExpr = Result.Nodes.getNodeAs<Expr>("redundantExpr"); const auto *OriginalExpr = Result.Nodes.getNodeAs<Expr>("originalStringView"); - assert(ParamExpr && RedundantExpr && OriginalExpr); + assert(StringView && RedundantExpr && OriginalExpr); bool IsCStrPattern = false; StringRef MethodName; @@ -109,14 +89,6 @@ void StringViewConversionsCheck::check(const MatchFinder::MatchResult &Result) { IsCStrPattern = true; } - // Sanity check. Verify that the redundant expression is the direct source of - // the argument, not part of a larger expression (e.g., std::string(sv) + - // "bar"). - // FIXME: This is a temporary solution to avoid assertions. Instead the - // matcher must be fixed. - if (ParamExpr->getSourceRange() != RedundantExpr->getSourceRange()) - return; - const StringRef OriginalText = Lexer::getSourceText( CharSourceRange::getTokenRange(OriginalExpr->getSourceRange()), *Result.SourceManager, getLangOpts()); @@ -131,12 +103,12 @@ void StringViewConversionsCheck::check(const MatchFinder::MatchResult &Result) { diag(RedundantExpr->getBeginLoc(), "redundant conversion to %0 and calling .%1() and then back to %2") << CStrCall->getImplicitObjectArgument()->getType() << MethodName - << ParamExpr->getType() << FixRedundantConversion; + << StringView->getType() << FixRedundantConversion; } else { // Handle direct std::string(sv) pattern diag(RedundantExpr->getBeginLoc(), "redundant conversion to %0 and then back to %1") - << RedundantExpr->getType() << ParamExpr->getType() + << RedundantExpr->getType() << StringView->getType() << FixRedundantConversion; } } diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/string-view-conversions.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/string-view-conversions.cpp index 531742e0ccd86..63980e7849fb3 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance/string-view-conversions.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/string-view-conversions.cpp @@ -42,6 +42,15 @@ std::string foo_str(int p1); std::wstring foo_wstr(int, const std::string&); std::string_view foo_sv(int p1); +struct TakesStringView { + TakesStringView(int, std::string_view); +}; + +struct StringBuilder { + StringBuilder& operator+=(std::string_view); + StringBuilder& append(std::string_view); +}; + void positive(std::string_view sv, std::wstring_view wsv) { // string(string_view) // @@ -109,6 +118,43 @@ void positive(std::string_view sv, std::wstring_view wsv) { foo_wsv(42, std::wstring(wptr), 3.14); // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: redundant conversion to 'std::wstring' (aka 'basic_string<wchar_t>') and then back to 'basic_string_view<wchar_t, std::char_traits<wchar_t>>' [performance-string-view-conversions] // CHECK-FIXES: foo_wsv(42, wptr, 3.14); + + TakesStringView(0, std::string("foo")); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: redundant conversion to 'std::string' (aka 'basic_string<char>') and then back to 'basic_string_view<char, std::char_traits<char>>' [performance-string-view-conversions] + // CHECK-FIXES: TakesStringView(0, "foo"); + + TakesStringView var(0, std::string("foo")); + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: redundant conversion to 'std::string' (aka 'basic_string<char>') and then back to 'basic_string_view<char, std::char_traits<char>>' [performance-string-view-conversions] + // CHECK-FIXES: TakesStringView var(0, "foo"); + + TakesStringView(1, std::string{"foo"}); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: redundant conversion to 'std::string' (aka 'basic_string<char>') and then back to 'basic_string_view<char, std::char_traits<char>>' [performance-string-view-conversions] + // CHECK-FIXES: TakesStringView(1, "foo"); + + TakesStringView(2, std::string(std::string_view("foo"))); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: redundant conversion to 'std::string' (aka 'basic_string<char>') and then back to 'basic_string_view<char, std::char_traits<char>>' [performance-string-view-conversions] + // CHECK-FIXES: TakesStringView(2, std::string_view("foo")); + + TakesStringView(3, std::string(foo_sv(42))); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: redundant conversion to 'std::string' (aka 'basic_string<char>') and then back to 'basic_string_view<char, std::char_traits<char>>' [performance-string-view-conversions] + // CHECK-FIXES: TakesStringView(3, foo_sv(42)); + + StringBuilder builder; + builder += std::string("hmm"); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: redundant conversion to 'std::string' (aka 'basic_string<char>') and then back to 'basic_string_view<char, std::char_traits<char>>' [performance-string-view-conversions] + // CHECK-FIXES: builder += "hmm"; + + builder.append(std::string("hmm")); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: redundant conversion to 'std::string' (aka 'basic_string<char>') and then back to 'basic_string_view<char, std::char_traits<char>>' [performance-string-view-conversions] + // CHECK-FIXES: builder.append("hmm"); + + std::string_view{std::string(foo_sv(1))}; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: redundant conversion to 'std::string' (aka 'basic_string<char>') and then back to 'basic_string_view<char, std::char_traits<char>>' [performance-string-view-conversions] + // CHECK-FIXES: std::string_view{foo_sv(1)}; + + std::string_view(std::string("hmm").data()); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: redundant conversion to 'const std::basic_string<char>' and calling .data() and then back to 'std::string_view' (aka 'basic_string_view<char>') [performance-string-view-conversions] + // CHECK-FIXES: std::string_view("hmm"); } void negative(std::string_view sv, std::wstring_view wsv) { @@ -142,6 +188,7 @@ void negative(std::string_view sv, std::wstring_view wsv) { // Move semantics ignored std::string s; foo_sv(42, std::move(s), 3.14); + TakesStringView{0, std::move(s)}; // Inner calls are ignored foo_wsv(foo_wstr(42, "Hello, world")); @@ -149,4 +196,5 @@ void negative(std::string_view sv, std::wstring_view wsv) { // No warnings expected: string parameter of a limited length, not string-view foo_sv(142, std::string("Hello, world", 5), 3.14); + TakesStringView{0, std::string("Hello, world", 5)}; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
