[clang-tools-extra] [clang-tidy] Simplify and generalize `performance-string-view-conversions` (PR #182783)
Victor Chernyakin via cfe-commits
cfe-commits at lists.llvm.org
Sun Feb 22 15:42:22 PST 2026
https://github.com/localspook created https://github.com/llvm/llvm-project/pull/182783
This change teaches the check to detect the pointless `string_view -> string -> string_view` conversion chain in places besides function calls (notably, in constructor calls), all with less code. It also solves the crash which prompted the temporary fix introduced by #179027.
No release note, because this check hasn't been released yet.
>From 5b34a4b5026e39587618d2f22ed964e06839d906 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
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 | 40 ++++++++++
2 files changed, 65 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..a5f62ab350320 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,35 @@ 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");
}
void negative(std::string_view sv, std::wstring_view wsv) {
@@ -142,6 +180,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 +188,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)};
}
More information about the cfe-commits
mailing list