[clang-tools-extra] [clang-tidy] perf-str-view-conv improvement: support c_str/data fixing (PR #181473)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Feb 14 04:32:24 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Zinovy Nis (irishrover)
<details>
<summary>Changes</summary>
---
Full diff: https://github.com/llvm/llvm-project/pull/181473.diff
3 Files Affected:
- (modified) clang-tools-extra/clang-tidy/performance/StringViewConversionsCheck.cpp (+43-13)
- (modified) clang-tools-extra/docs/clang-tidy/checks/performance/string-view-conversions.rst (+5)
- (modified) clang-tools-extra/test/clang-tidy/checkers/performance/string-view-conversions.cpp (+12-1)
``````````diff
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);
``````````
</details>
https://github.com/llvm/llvm-project/pull/181473
More information about the cfe-commits
mailing list