[clang-tools-extra] [clang-tidy] Add performance-substr-self-assignment check (PR #209657)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 14 18:03:32 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Andrew Gaul (gaul)
<details>
<summary>Changes</summary>
Finds `s = s.substr(...)` self-assignments that materialize an unnecessary temporary string and rewrites the prefix-stripping forms (`substr(pos)`, `substr(pos, npos)`) to an in-place `s.erase(0, pos)`. The truncation form `s = s.substr(0, count)` is diagnosed without a fix-it because `s.erase(count)` throws std::out_of_range where `substr` clamps.
---
Full diff: https://github.com/llvm/llvm-project/pull/209657.diff
9 Files Affected:
- (modified) clang-tools-extra/clang-tidy/performance/CMakeLists.txt (+1)
- (modified) clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp (+3)
- (added) clang-tools-extra/clang-tidy/performance/SubstrSelfAssignmentCheck.cpp (+128)
- (added) clang-tools-extra/clang-tidy/performance/SubstrSelfAssignmentCheck.h (+44)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+7)
- (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1)
- (added) clang-tools-extra/docs/clang-tidy/checks/performance/substr-self-assignment.rst (+45)
- (modified) clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/string (+1)
- (added) clang-tools-extra/test/clang-tidy/checkers/performance/substr-self-assignment.cpp (+154)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
index 0c778b5a9f36b..6851daca87028 100644
--- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
@@ -22,6 +22,7 @@ add_clang_library(clangTidyPerformanceModule STATIC
PerformanceTidyModule.cpp
PreferSingleCharOverloadsCheck.cpp
StringViewConversionsCheck.cpp
+ SubstrSelfAssignmentCheck.cpp
TriviallyDestructibleCheck.cpp
TypePromotionInMathFnCheck.cpp
UnnecessaryCopyInitializationCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
index a4c1cdacab496..558fd56f19777 100644
--- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
@@ -24,6 +24,7 @@
#include "NoexceptSwapCheck.h"
#include "PreferSingleCharOverloadsCheck.h"
#include "StringViewConversionsCheck.h"
+#include "SubstrSelfAssignmentCheck.h"
#include "TriviallyDestructibleCheck.h"
#include "TypePromotionInMathFnCheck.h"
#include "UnnecessaryCopyInitializationCheck.h"
@@ -68,6 +69,8 @@ class PerformanceModule : public ClangTidyModule {
"performance-prefer-single-char-overloads");
CheckFactories.registerCheck<StringViewConversionsCheck>(
"performance-string-view-conversions");
+ CheckFactories.registerCheck<SubstrSelfAssignmentCheck>(
+ "performance-substr-self-assignment");
CheckFactories.registerCheck<TriviallyDestructibleCheck>(
"performance-trivially-destructible");
CheckFactories.registerCheck<TypePromotionInMathFnCheck>(
diff --git a/clang-tools-extra/clang-tidy/performance/SubstrSelfAssignmentCheck.cpp b/clang-tools-extra/clang-tidy/performance/SubstrSelfAssignmentCheck.cpp
new file mode 100644
index 0000000000000..4d1b9cd4da149
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/SubstrSelfAssignmentCheck.cpp
@@ -0,0 +1,128 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "SubstrSelfAssignmentCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include <optional>
+#include <string>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::performance {
+
+// Returns true if E refers to the static member 'npos' of Class. A bare name
+// comparison is not enough: a local variable or parameter that happens to be
+// called 'npos' carries a real count, which has no single-'erase' rewrite.
+static bool isNposOfClass(const Expr *E, const CXXRecordDecl *Class) {
+ const ValueDecl *D = nullptr;
+ E = E->IgnoreParenImpCasts();
+ if (const auto *Ref = dyn_cast<DeclRefExpr>(E))
+ D = Ref->getDecl();
+ else if (const auto *Member = dyn_cast<MemberExpr>(E))
+ D = Member->getMemberDecl();
+ const auto *Var = dyn_cast_or_null<VarDecl>(D);
+ if (!Var || !Var->isStaticDataMember() || Var->getName() != "npos")
+ return false;
+ return declaresSameEntity(dyn_cast<CXXRecordDecl>(Var->getDeclContext()),
+ Class);
+}
+
+SubstrSelfAssignmentCheck::SubstrSelfAssignmentCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ StringLikeClasses(utils::options::parseStringList(
+ Options.get("StringLikeClasses", "::std::basic_string"))) {}
+
+void SubstrSelfAssignmentCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "StringLikeClasses",
+ utils::options::serializeStringList(StringLikeClasses));
+}
+
+void SubstrSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
+ const auto VarRef =
+ ignoringParens(declRefExpr(to(varDecl().bind("var"))).bind("lhs"));
+
+ const auto SubstrCall =
+ cxxMemberCallExpr(
+ callee(cxxMethodDecl(hasName("substr"),
+ ofClass(hasAnyName(StringLikeClasses)))),
+ on(ignoringParens(declRefExpr(to(varDecl(equalsBoundNode("var")))))))
+ .bind("substr");
+
+ // Match: s = s.substr(...)
+ Finder->addMatcher(cxxOperatorCallExpr(hasOperatorName("="),
+ hasArgument(0, VarRef),
+ hasArgument(1, SubstrCall))
+ .bind("assign"),
+ this);
+}
+
+void SubstrSelfAssignmentCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *AssignExpr =
+ Result.Nodes.getNodeAs<CXXOperatorCallExpr>("assign");
+ const auto *LHS = Result.Nodes.getNodeAs<DeclRefExpr>("lhs");
+ const auto *SubstrExpr = Result.Nodes.getNodeAs<CXXMemberCallExpr>("substr");
+ const SourceManager &SM = *Result.SourceManager;
+ const LangOptions &LangOpts = Result.Context->getLangOpts();
+
+ const CXXMethodDecl *Method = SubstrExpr->getMethodDecl();
+ if (!Method)
+ return;
+
+ // Count only explicitly-written arguments (exclude CXXDefaultArgExpr).
+ unsigned NumExplicitArgs = 0;
+ for (const Expr *Arg : SubstrExpr->arguments())
+ if (!isa<CXXDefaultArgExpr>(Arg))
+ ++NumExplicitArgs;
+
+ // s = s.substr(pos) and s = s.substr(pos, npos) strip a prefix and can be
+ // rewritten as s.erase(0, pos). The truncation form s = s.substr(0, count)
+ // is diagnosed without a fix-it: s.erase(count) throws std::out_of_range
+ // for count > s.size() where substr merely clamps. The general
+ // two-argument form has no single-'erase' equivalent and is ignored.
+ const bool IsPrefixStrip =
+ NumExplicitArgs == 1 ||
+ (NumExplicitArgs == 2 &&
+ isNposOfClass(SubstrExpr->getArg(1), Method->getParent()));
+ bool IsTruncation = false;
+ if (!IsPrefixStrip && NumExplicitArgs == 2) {
+ const auto *PosLiteral =
+ dyn_cast<IntegerLiteral>(SubstrExpr->getArg(0)->IgnoreParenImpCasts());
+ IsTruncation = PosLiteral && PosLiteral->getValue() == 0;
+ }
+ if (!IsPrefixStrip && !IsTruncation)
+ return;
+
+ // Rewriting a macro expansion is unsafe; emit the warning without a fix-it.
+ std::optional<std::string> Replacement;
+ if (IsPrefixStrip && !AssignExpr->getBeginLoc().isMacroID() &&
+ !AssignExpr->getEndLoc().isMacroID()) {
+ StringRef VarName = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(LHS->getSourceRange()), SM, LangOpts);
+ StringRef PosText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(SubstrExpr->getArg(0)->getSourceRange()),
+ SM, LangOpts);
+ if (!VarName.empty() && !PosText.empty())
+ Replacement = (VarName + ".erase(0, " + PosText + ")").str();
+ }
+
+ auto Diag = diag(AssignExpr->getOperatorLoc(),
+ "inefficient self-assignment via 'substr'; use 'erase' to "
+ "modify the string in-place");
+ if (Replacement)
+ Diag << FixItHint::CreateReplacement(AssignExpr->getSourceRange(),
+ *Replacement);
+}
+
+} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/clang-tidy/performance/SubstrSelfAssignmentCheck.h b/clang-tools-extra/clang-tidy/performance/SubstrSelfAssignmentCheck.h
new file mode 100644
index 0000000000000..cf9dc425c44d2
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/SubstrSelfAssignmentCheck.h
@@ -0,0 +1,44 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_SUBSTRSELFASSIGNMENTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_SUBSTRSELFASSIGNMENTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+#include <vector>
+
+namespace clang::tidy::performance {
+
+/// Finds cases where a string variable is assigned the result of calling
+/// ``substr()`` on itself (e.g., ``s = s.substr(x, y)``). This pattern creates
+/// an unnecessary temporary string; the same effect can be achieved in-place
+/// using ``erase()``.
+///
+/// For the user-facing documentation see:
+/// https://clang.llvm.org/extra/clang-tidy/checks/performance/substr-self-assignment.html
+class SubstrSelfAssignmentCheck : public ClangTidyCheck {
+public:
+ SubstrSelfAssignmentCheck(StringRef Name, ClangTidyContext *Context);
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
+ std::optional<TraversalKind> getCheckTraversalKind() const override {
+ return TK_IgnoreUnlessSpelledInSource;
+ }
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+ const std::vector<StringRef> StringLikeClasses;
+};
+
+} // namespace clang::tidy::performance
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_SUBSTRSELFASSIGNMENTCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 69c3bcf67b8db..6dbcf2eb48b4c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -97,6 +97,13 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
+- New :doc:`performance-substr-self-assignment
+ <clang-tidy/checks/performance/substr-self-assignment>` check.
+
+ Finds cases where a string variable is assigned the result of calling
+ ``substr()`` on itself, which creates an unnecessary temporary. Suggests
+ using ``erase()`` to modify the string in-place instead.
+
New check aliases
^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 2a44dc78fbc89..54e2b0d69b2e5 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -367,6 +367,7 @@ Clang-Tidy Checks
:doc:`performance-noexcept-swap <performance/noexcept-swap>`, "Yes"
:doc:`performance-prefer-single-char-overloads <performance/prefer-single-char-overloads>`, "Yes"
:doc:`performance-string-view-conversions <performance/string-view-conversions>`, "Yes"
+ :doc:`performance-substr-self-assignment <performance/substr-self-assignment>`, "Yes"
:doc:`performance-trivially-destructible <performance/trivially-destructible>`, "Yes"
:doc:`performance-type-promotion-in-math-fn <performance/type-promotion-in-math-fn>`, "Yes"
:doc:`performance-unnecessary-copy-initialization <performance/unnecessary-copy-initialization>`, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/substr-self-assignment.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/substr-self-assignment.rst
new file mode 100644
index 0000000000000..d71030dc15ae1
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/substr-self-assignment.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - performance-substr-self-assignment
+
+performance-substr-self-assignment
+===================================
+
+Finds cases where a string variable is assigned the result of calling
+``substr()`` on itself. This pattern materializes an unnecessary temporary
+string (an allocation, a copy of the surviving characters, and a
+deallocation) and discards the original capacity; the same effect can be
+achieved in-place with ``erase()``.
+
+.. code-block:: c++
+
+ std::string s = "hello world";
+
+ s = s.substr(5); // warning; fix-it: s.erase(0, 5)
+ s = s.substr(5, std::string::npos); // warning; fix-it: s.erase(0, 5)
+ s = s.substr(0, 3); // warning; no fix-it (see below)
+
+The fix-it for the prefix-stripping forms replaces the assignment with
+``s.erase(0, pos)``. The two expressions differ only when
+``pos > s.size()``: ``substr`` throws ``std::out_of_range``, while
+``erase(0, pos)`` clamps and erases the whole string. Code that relies on
+that exception changes behavior under the fix-it.
+
+The truncation form ``s = s.substr(0, count)`` is diagnosed without a
+fix-it: the tempting replacement ``s.erase(count)`` throws
+``std::out_of_range`` whenever ``count > s.size()``, whereas ``substr``
+clamps ``count`` and leaves the string unchanged. A safe manual rewrite is
+``s.resize(std::min(count, s.size()))``.
+
+No diagnostic is emitted for the general form ``s = s.substr(pos, count)``,
+which has no single-call in-place equivalent. Inside macro expansions the
+warning is emitted without a fix-it. Only self-assignments to plain
+variables are diagnosed; assignments through class members or pointers are
+not.
+
+Options
+-------
+
+.. option:: StringLikeClasses
+
+ Semicolon-separated list of names of string-like classes. By default only
+ ``::std::basic_string`` is considered. Classes listed here must provide
+ ``substr``, ``erase``, and ``npos`` with ``std::basic_string`` semantics.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/string b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/string
index dbebeaaa46514..edb7dc587d865 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/string
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/string
@@ -75,6 +75,7 @@ struct basic_string {
_Type& insert(size_type pos, const C* s);
_Type& insert(size_type pos, const C* s, size_type n);
+ _Type& erase(size_type pos = 0, size_type count = npos);
_Type substr(size_type pos = 0, size_type count = npos) const;
constexpr bool starts_with(std::basic_string_view<C, T> sv) const noexcept;
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/substr-self-assignment.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/substr-self-assignment.cpp
new file mode 100644
index 0000000000000..0ad421ffd98f3
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/substr-self-assignment.cpp
@@ -0,0 +1,154 @@
+// RUN: %check_clang_tidy %s performance-substr-self-assignment %t
+#include <string>
+
+void OneArg() {
+ std::string s = "hello world";
+
+ // Basic case: s = s.substr(pos)
+ s = s.substr(5);
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: inefficient self-assignment via 'substr'; use 'erase' to modify the string in-place [performance-substr-self-assignment]
+ // CHECK-FIXES: s.erase(0, 5);
+
+ // With a variable as the argument.
+ int pos = 3;
+ s = s.substr(pos);
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: inefficient self-assignment via 'substr'
+ // CHECK-FIXES: s.erase(0, pos);
+
+ // With a more complex expression.
+ s = s.substr(pos + 1);
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: inefficient self-assignment via 'substr'
+ // CHECK-FIXES: s.erase(0, pos + 1);
+}
+
+void TruncationNoFixIt() {
+ std::string s = "hello world";
+
+ // The truncation form s = s.substr(0, count) is diagnosed but gets no
+ // fix-it: 'erase(count)' throws std::out_of_range when count > size(),
+ // while 'substr(0, count)' clamps.
+ s = s.substr(0, 5);
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: inefficient self-assignment via 'substr'; use 'erase' to modify the string in-place [performance-substr-self-assignment]
+ // CHECK-FIXES: s = s.substr(0, 5);
+
+ size_t count = 3;
+ s = s.substr(0, count);
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: inefficient self-assignment via 'substr'
+ // CHECK-FIXES: s = s.substr(0, count);
+}
+
+void TwoArgWithNpos() {
+ std::string s = "hello world";
+
+ // s = s.substr(pos, npos) is equivalent to s = s.substr(pos).
+ s = s.substr(3, std::string::npos);
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: inefficient self-assignment via 'substr'; use 'erase' to modify the string in-place [performance-substr-self-assignment]
+ // CHECK-FIXES: s.erase(0, 3);
+
+ int pos = 2;
+ s = s.substr(pos, std::string::npos);
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: inefficient self-assignment via 'substr'
+ // CHECK-FIXES: s.erase(0, pos);
+
+ // Member-syntax spelling of npos.
+ s = s.substr(4, s.npos);
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: inefficient self-assignment via 'substr'
+ // CHECK-FIXES: s.erase(0, 4);
+}
+
+void LocalVariableNamedNpos() {
+ std::string s = "hello world";
+
+ // A local variable named 'npos' is not std::basic_string::npos; the count
+ // is meaningful, so there is no single-'erase' rewrite and no warning.
+ size_t npos = 3;
+ s = s.substr(2, npos);
+}
+
+void ParamNamedNpos(std::string s, std::string::size_type npos) {
+ s = s.substr(1, npos);
+}
+
+void TwoArgGeneral() {
+ std::string s = "hello world";
+
+ // General two-argument case: no diagnostic (no single erase() equivalent).
+ s = s.substr(2, 3);
+
+ int pos = 1;
+ size_t len = 4;
+ s = s.substr(pos, len);
+}
+
+void Negatives() {
+ std::string s = "hello";
+ std::string t = "world";
+
+ // Different variables -- not a self-assignment.
+ s = t.substr(1);
+ s = t.substr(0, 3);
+
+ // Not an assignment to the same variable.
+ std::string r = s.substr(1);
+}
+
+struct Holder {
+ std::string Str;
+ // Self-assignment through a class member is not matched; only plain
+ // variables are.
+ void trim() { Str = Str.substr(2); }
+};
+
+template <typename T>
+void dependentType(T t) {
+ // Type-dependent: no diagnostic, including in instantiations.
+ t = t.substr(1);
+}
+void instantiate() { dependentType(std::string("hello")); }
+
+void WideString() {
+ std::wstring ws = L"hello world";
+
+ ws = ws.substr(3);
+ // CHECK-MESSAGES: [[@LINE-1]]:6: warning: inefficient self-assignment via 'substr'
+ // CHECK-FIXES: ws.erase(0, 3);
+
+ // Truncation form: warning only, no fix-it.
+ ws = ws.substr(0, 5);
+ // CHECK-MESSAGES: [[@LINE-1]]:6: warning: inefficient self-assignment via 'substr'
+ // CHECK-FIXES: ws = ws.substr(0, 5);
+}
+
+void Parenthesized() {
+ std::string s = "hello";
+
+ // Parenthesized object on the RHS -- should still match.
+ s = (s).substr(2);
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: inefficient self-assignment via 'substr'
+ // CHECK-FIXES: s.erase(0, 2);
+
+ // Parenthesized substr call on the RHS.
+ s = (s.substr(3));
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: inefficient self-assignment via 'substr'
+ // CHECK-FIXES: s.erase(0, 3);
+}
+
+#define STRIP_PREFIX(str, n) str = str.substr(n)
+void MacroExpansion() {
+ std::string s = "hello";
+
+ // Diagnosed, but no fix-it: rewriting a macro expansion is unsafe.
+ STRIP_PREFIX(s, 2);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: inefficient self-assignment via 'substr'
+ // CHECK-FIXES: STRIP_PREFIX(s, 2);
+}
+
+#define OFFSET 2
+void MacroArgument() {
+ std::string s = "hello";
+
+ // Only the argument comes from a macro; the fix-it preserves its spelling.
+ s = s.substr(OFFSET);
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: inefficient self-assignment via 'substr'
+ // CHECK-FIXES: s.erase(0, OFFSET);
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/209657
More information about the cfe-commits
mailing list