https://github.com/irishrover created https://github.com/llvm/llvm-project/pull/181473
None >From c7341bc0d3f0f1071a08f3acedb7dafbcb66a1fe Mon Sep 17 00:00:00 2001 From: Zinovy Nis <[email protected]> Date: Sat, 14 Feb 2026 15:26:00 +0300 Subject: [PATCH] [clang-tidy] perf-str-view-conv improvement: support c_str/data fixing --- .../StringViewConversionsCheck.cpp | 56 ++++++++++++++----- .../performance/string-view-conversions.rst | 5 ++ .../performance/string-view-conversions.cpp | 13 ++++- 3 files changed, 60 insertions(+), 14 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/StringViewConversionsCheck.cpp b/clang-tools-extra/clang-tidy/performance/StringViewConversionsCheck.cpp index aebbec5e9b913..6ca5c50b10c50 100644 --- a/clang-tools-extra/clang-tidy/performance/StringViewConversionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/StringViewConversionsCheck.cpp @@ -12,13 +12,14 @@ #include "clang/Lex/Lexer.h" using namespace clang::ast_matchers; - namespace clang::tidy::performance { 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"); @@ -48,18 +49,25 @@ 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(), c_str(), etc. + // 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)))); + 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))); + // 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 @@ -72,11 +80,14 @@ void StringViewConversionsCheck::registerMatchers(MatchFinder *Finder) { expr(hasType(IsStdStringView), // Ignore cases where the argument is a function call unless(ignoringParenImpCasts(IsImplicitStringViewFromCall)), - // Match either syntax for std::string construction + // Match either syntax for std::string construction or + // .c_str()/.data() pattern hasDescendant(expr(anyOf(RedundantFunctionalCast, - RedundantStringConstruction)) + 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)))), @@ -87,7 +98,16 @@ void StringViewConversionsCheck::check(const MatchFinder::MatchResult &Result) { const auto *ParamExpr = Result.Nodes.getNodeAs<Expr>("paramExpr"); const auto *RedundantExpr = Result.Nodes.getNodeAs<Expr>("redundantExpr"); const auto *OriginalExpr = Result.Nodes.getNodeAs<Expr>("originalStringView"); - assert(RedundantExpr && ParamExpr && OriginalExpr); + assert(ParamExpr && RedundantExpr && OriginalExpr); + + bool IsCStrPattern = false; + StringRef MethodName; + const auto *CStrCall = dyn_cast<CXXMemberCallExpr>(RedundantExpr); + if (CStrCall && CStrCall->getMethodDecl()) { + MethodName = CStrCall->getMethodDecl()->getName(); + if (MethodName == "c_str" || MethodName == "data") + 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) + @@ -104,11 +124,21 @@ void StringViewConversionsCheck::check(const MatchFinder::MatchResult &Result) { if (OriginalText.empty()) return; - diag(RedundantExpr->getBeginLoc(), - "redundant conversion to %0 and then back to %1") - << RedundantExpr->getType() << ParamExpr->getType() - << FixItHint::CreateReplacement(RedundantExpr->getSourceRange(), - OriginalText); + FixItHint FixRedundantConversion = FixItHint::CreateReplacement( + RedundantExpr->getSourceRange(), OriginalText); + if (IsCStrPattern && CStrCall) { + // Handle std::string(sv).c_str() or std::string(sv).data() pattern + diag(RedundantExpr->getBeginLoc(), + "redundant conversion to %0 and calling .%1() and then back to %2") + << CStrCall->getImplicitObjectArgument()->getType() << MethodName + << ParamExpr->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() + << FixRedundantConversion; + } } } // namespace clang::tidy::performance diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/string-view-conversions.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/string-view-conversions.rst index 8d673ea1c6e1b..a134b47e95e32 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/performance/string-view-conversions.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/performance/string-view-conversions.rst @@ -15,6 +15,10 @@ Before: foo(42, std::string(sv), 3.14); // conversion to std::string is // redundant as std::string_view // is expected + foo(42, std::string(sv).c_str(), 2.71); // conversion to std::string and + // then to char* is redundant + // as std::string_view + // is expected foo(42, std::string("foo"), 3.14); // conversion to std::string is // redundant as std::string_view // is expected @@ -27,5 +31,6 @@ After: void foo(int p1, std::string_view p2, double p3); void bar(std::string_view sv) { foo(42, sv, 3.14); + foo(42, sv, 2.71); foo(42, "foo", 3.14); } 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 69365a064c611..531742e0ccd86 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 @@ -49,6 +49,14 @@ void positive(std::string_view sv, std::wstring_view wsv) { // 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: foo_sv(42, sv, 3.14); + foo_sv(42, std::string(sv).c_str(), 3.14); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: redundant conversion to 'const std::basic_string<char>' and calling .c_str() and then back to 'std::string_view' (aka 'basic_string_view<char>') [performance-string-view-conversions] + // CHECK-FIXES: foo_sv(42, sv, 3.14); + + foo_sv(42, std::string(sv).data(), 3.14); + // CHECK-MESSAGES: :[[@LINE-1]]:14: 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: foo_sv(42, sv, 3.14); + foo_sv(42, std::string("Hello, world"), 3.14); // 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: foo_sv(42, "Hello, world", 3.14); @@ -93,6 +101,10 @@ void positive(std::string_view sv, std::wstring_view wsv) { // 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, wsv, 3.14); + foo_wsv(42, std::wstring(wsv).c_str(), 3.14); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: redundant conversion to 'const std::basic_string<wchar_t>' and calling .c_str() and then back to 'std::wstring_view' (aka 'basic_string_view<wchar_t>') [performance-string-view-conversions] + // CHECK-FIXES: foo_wsv(42, wsv, 3.14); + const wchar_t *wptr = L"Hello, world"; 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] @@ -117,7 +129,6 @@ void negative(std::string_view sv, std::wstring_view wsv) { foo_sv(42, std::string(5, 'a'), 3.14); foo_sv(42, std::string("foo").append("bar"), 3.14); foo_sv(42, std::string(sv).substr(0, 5), 3.14); - foo_sv(42, std::string(sv).c_str(), 3.14); // No warnings expected: string parameter, not string-view foo_str(42, std::string(sv), 3.14); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
