https://github.com/irishrover updated https://github.com/llvm/llvm-project/pull/172170
>From a6c418cee372d84760131b742e36efabda7310e2 Mon Sep 17 00:00:00 2001 From: Zinovy Nis <[email protected]> Date: Sat, 13 Dec 2025 13:47:01 +0300 Subject: [PATCH] [clang-tidy] Add a new check 'replace-with-string-view' Looks for functions returning `std::[w|u8|u16|u32]string` and suggests to change it to `std::[...]string_view` if possible and profitable. Example: ```cpp std::string foo(int i) { // <---- can be replaced to `std::string_view foo(...) {` switch(i) { case 1: return "case1"; case 2: return "case2"; default: return {}; } } ``` --- .../clang-tidy/performance/CMakeLists.txt | 1 + .../performance/PerformanceTidyModule.cpp | 3 + .../ReplaceWithStringViewCheck.cpp | 72 +++++++++ .../performance/ReplaceWithStringViewCheck.h | 47 ++++++ clang-tools-extra/docs/ReleaseNotes.rst | 6 + .../docs/clang-tidy/checks/list.rst | 1 + .../performance/replace-with-string-view.rst | 60 ++++++++ .../performance/replace-with-string-view.cpp | 143 ++++++++++++++++++ 8 files changed, 333 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/performance/ReplaceWithStringViewCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/performance/ReplaceWithStringViewCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/performance/replace-with-string-view.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/replace-with-string-view.cpp diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt index 9a2f90069edbf..21a9442be69c0 100644 --- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt @@ -21,6 +21,7 @@ add_clang_library(clangTidyPerformanceModule STATIC NoexceptMoveConstructorCheck.cpp NoexceptSwapCheck.cpp PerformanceTidyModule.cpp + ReplaceWithStringViewCheck.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 3497ea7316c6b..e17bfc42be58f 100644 --- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp @@ -24,6 +24,7 @@ #include "NoexceptDestructorCheck.h" #include "NoexceptMoveConstructorCheck.h" #include "NoexceptSwapCheck.h" +#include "ReplaceWithStringViewCheck.h" #include "TriviallyDestructibleCheck.h" #include "TypePromotionInMathFnCheck.h" #include "UnnecessaryCopyInitializationCheck.h" @@ -62,6 +63,8 @@ class PerformanceModule : public ClangTidyModule { "performance-noexcept-move-constructor"); CheckFactories.registerCheck<NoexceptSwapCheck>( "performance-noexcept-swap"); + CheckFactories.registerCheck<ReplaceWithStringViewCheck>( + "performance-replace-with-string-view"); CheckFactories.registerCheck<TriviallyDestructibleCheck>( "performance-trivially-destructible"); CheckFactories.registerCheck<TypePromotionInMathFnCheck>( diff --git a/clang-tools-extra/clang-tidy/performance/ReplaceWithStringViewCheck.cpp b/clang-tools-extra/clang-tidy/performance/ReplaceWithStringViewCheck.cpp new file mode 100644 index 0000000000000..08d0a15f7a2ab --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/ReplaceWithStringViewCheck.cpp @@ -0,0 +1,72 @@ +//===----------------------------------------------------------------------===// +// +// 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 "ReplaceWithStringViewCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::performance { + +static llvm::StringLiteral toStringViewTypeStr(StringRef Type) { + if (Type.contains("wchar_t")) + return "std::wstring_view"; + if (Type.contains("wchar_t")) + return "std::wstring_view"; + if (Type.contains("char8_t")) + return "std::u8string_view"; + if (Type.contains("char16_t")) + return "std::u16string_view"; + if (Type.contains("char32_t")) + return "std::u32string_view"; + return "std::string_view"; +} + +void ReplaceWithStringViewCheck::registerMatchers(MatchFinder *Finder) { + const auto IsStdString = hasCanonicalType( + hasDeclaration(cxxRecordDecl(hasName("::std::basic_string")))); + const auto IsStdStringView = expr(hasType(hasCanonicalType( + hasDeclaration(cxxRecordDecl(hasName("::std::basic_string_view")))))); + + Finder->addMatcher( + functionDecl( + isDefinition(), returns(IsStdString), hasDescendant(returnStmt()), + unless(hasDescendant( + returnStmt(hasReturnValue(unless(ignoringImplicit(anyOf( + stringLiteral(), IsStdStringView, + cxxConstructExpr( + hasType(IsStdString), + anyOf(argumentCountIs(0), + hasArgument(0, ignoringParenImpCasts(anyOf( + stringLiteral(), + IsStdStringView))))))))))))) + .bind("func"), + this); +} + +void ReplaceWithStringViewCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("func")) { + const llvm::StringLiteral DestReturnTypeStr = + toStringViewTypeStr(MatchedDecl->getReturnType() + .getDesugaredType(*Result.Context) + .getAsString()); + + auto Diag = diag(MatchedDecl->getReturnTypeSourceRange().getBegin(), + "consider using `%0' to avoid unnecessary " + "copying and allocations") + << DestReturnTypeStr << MatchedDecl; + + for (const auto *FuncDecl = MatchedDecl; FuncDecl != nullptr; + FuncDecl = FuncDecl->getPreviousDecl()) { + Diag << FixItHint::CreateReplacement(FuncDecl->getReturnTypeSourceRange(), + DestReturnTypeStr); + } + } +} + +} // namespace clang::tidy::performance diff --git a/clang-tools-extra/clang-tidy/performance/ReplaceWithStringViewCheck.h b/clang-tools-extra/clang-tidy/performance/ReplaceWithStringViewCheck.h new file mode 100644 index 0000000000000..a9635939e850d --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/ReplaceWithStringViewCheck.h @@ -0,0 +1,47 @@ +//===----------------------------------------------------------------------===// +// +// 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_REPLACEWITHSTRINGVIEWCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_REPLACEWITHSTRINGVIEWCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::performance { + +/// Looks for functions returning `std::[w|u8|u16|u32]string` and suggests to +/// change it to `std::[...]string_view` if possible and profitable. +/// +/// Example: +/// +/// std::string foo(int i) { // <---- can be replaced to std::string_view f(...) +/// switch(i) { +/// case 1: +/// return "case1"; +/// case 2: +/// return "case2"; +/// default: +/// return {}; +/// } +/// } +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/performance/replace-with-string-view.html +class ReplaceWithStringViewCheck : public ClangTidyCheck { +public: + ReplaceWithStringViewCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus17; + } +}; + +} // namespace clang::tidy::performance + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_REPLACEWITHSTRINGVIEWCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index afd873e4007ac..94abc9cfcd09d 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -244,6 +244,12 @@ New checks Finds virtual function overrides with different visibility than the function in the base class. +- New :doc:`performance-replace-with-string-view + <clang-tidy/checks/performance/replace-with-string-view>` check. + + Looks for functions returning `std::[w|u8|u16|u32]string` and suggests to + change it to `std::[...]string_view` for performance reasons if possible. + - New :doc:`readability-redundant-parentheses <clang-tidy/checks/readability/redundant-parentheses>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index e5e77b5cc418b..a45293b1f7823 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -362,6 +362,7 @@ Clang-Tidy Checks :doc:`performance-noexcept-destructor <performance/noexcept-destructor>`, "Yes" :doc:`performance-noexcept-move-constructor <performance/noexcept-move-constructor>`, "Yes" :doc:`performance-noexcept-swap <performance/noexcept-swap>`, "Yes" + :doc:`performance-replace-with-string-view <performance/replace-with-string-view>`, "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/replace-with-string-view.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/replace-with-string-view.rst new file mode 100644 index 0000000000000..6060c67b9fe67 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/performance/replace-with-string-view.rst @@ -0,0 +1,60 @@ +.. title:: clang-tidy - performance-replace-with-string-view + +performance-replace-with-string-view +==================================== + +Looks for functions returning `std::[w|u8|u16|u32]string` and suggests to +change it to `std::[...]string_view` for performance reasons if possible. + +Rationale: + +Each time a new ``std::string`` is created from a literal, a copy of that +literal is allocated either in ``std::string``'s internal buffer +(for short literals) or in a heap. + +For the cases where ``std::string`` is returned from a function, +such allocations can be eliminated sometimes by using ``std::string_view`` +as a return type. + +This check looks for such functions returning ``std::string`` +baked from the literals and suggests replacing the return type to ``std::string_view``. + +It handles ``std::string``, ``std::wstring``, ``std::u8string``, ``std::u16string`` and +``std::u32string`` and properly selects the kind of ``std::string_view`` to return. +Typedefs for these string type also supported. + +Example: + +Consider the following code: + +.. code-block:: c++ + + std::string foo(int i) { + switch(i) + { + case 1: return "case 1"; + case 2: return "case 2"; + case 3: return "case 3"; + default: return "default"; + } + } + +In the code above a new ``std::string`` object is created on each +function invocation, making a copy of a string literal and possibly +allocating a memory in a heap. + +The check gets this code transformed into: + +.. code-block:: c++ + + std::string_view foo(int i) { + switch(i) + { + case 1: return "case 1"; + case 2: return "case 2"; + case 3: return "case 3"; + default: return "default"; + } + } + +New version re-uses statically allocated literals without additional overhead. diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/replace-with-string-view.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/replace-with-string-view.cpp new file mode 100644 index 0000000000000..9c0de5aa0d4cb --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/replace-with-string-view.cpp @@ -0,0 +1,143 @@ +// RUN: %check_clang_tidy -std=c++17-or-later %s performance-replace-with-string-view %t -- -- -I %S/Inputs + +namespace std { + template <typename CharT> + class basic_string_view { + public: + basic_string_view(const CharT *); + basic_string_view(); + }; + using string_view = basic_string_view<char>; + using wstring_view = basic_string_view<wchar_t>; + using u16string_view = basic_string_view<char16_t>; + + template <typename CharT> + class basic_string { + public: + basic_string(); + basic_string(const CharT *); + basic_string(basic_string_view<CharT>); + + basic_string operator+(const basic_string &) const; + }; + using string = basic_string<char>; + using wstring = basic_string<wchar_t>; + using u16string = basic_string<char16_t>; +} + +// ========================================================== +// Positive tests +// ========================================================== + +std::string simpleLiteral() { +// CHECK-MESSAGES:[[@LINE-1]]:1: {{.*}}[performance-replace-with-string-view]{{.*}} +// CHECK-FIXES: std::string_view{{.*}} + return "simpleLiteral"; +} + +std::string implicitView(std::string_view sv) { +// CHECK-MESSAGES:[[@LINE-1]]:1: {{.*}}[performance-replace-with-string-view]{{.*}} +// CHECK-FIXES: std::string_view{{.*}} + return sv; +} + +std::string emptyReturn() { +// CHECK-MESSAGES:[[@LINE-1]]:1: {{.*}}[performance-replace-with-string-view]{{.*}} +// CHECK-FIXES: std::string_view{{.*}} + return {}; +} + +std::string switchCaseTest(int i) { +// CHECK-MESSAGES:[[@LINE-1]]:1: {{.*}}[performance-replace-with-string-view]{{.*}} +// CHECK-FIXES: std::string_view{{.*}} + switch (i) { + case 1: + return "case1"; + case 2: + return "case2"; + case 3: + return {}; + default: + return "default"; + } +} + +std::string ifElseTest(bool flag) { +// CHECK-MESSAGES:[[@LINE-1]]:1: {{.*}}[performance-replace-with-string-view]{{.*}} +// CHECK-FIXES: std::string_view{{.*}} + if (flag) + return "true"; + return "false"; +} + +std::wstring wideStringTest() { +// CHECK-MESSAGES:[[@LINE-1]]:1: {{.*}}[performance-replace-with-string-view]{{.*}} +// CHECK-FIXES: std::wstring_view{{.*}} + return L"wide literal"; +} + +std::u16string u16StringTest() { +// CHECK-MESSAGES:[[@LINE-1]]:1: {{.*}}[performance-replace-with-string-view]{{.*}} +// CHECK-FIXES: std::u16string_view{{.*}} + return u"u16 literal"; +} + +class A { + std::string classMethodInt() { return "internal"; } +// CHECK-MESSAGES:[[@LINE-1]]:3: {{.*}}[performance-replace-with-string-view]{{.*}} +// CHECK-FIXES: std::string_view{{.*}} + + std::string classMethodExt(); +// CHECK-MESSAGES:[[@LINE+4]]:1: {{.*}}[performance-replace-with-string-view]{{.*}} +// CHECK-FIXES: std::string_view{{.*}} +}; + +std::string A::classMethodExt() { return "external"; } +// CHECK-FIXES: std::string_view{{.*}} + +// ========================================================== +// Negative tests +// ========================================================== + +std::string localVariable() { + std::string s = "local variable"; + // TODO: extract and return literal + return s; +} + +std::string dynamicCalculation() { + std::string s1 = "hello "; + return s1 + "world"; +} + +std::string mixedReturns(bool flag) { + if (flag) { + return "safe static literal"; + } + std::string s = "unsafe dynamic"; + return s; +} + +std::string_view alreadyGood() { + return "alreadyGood"; +} + +std::string returnArgCopy(std::string s) { + return s; +} + +std::string returnIndirection(const char* ptr) { + // Can be unsafe or intentional, like converting string_view into string + return ptr; +} + +std::string passStringView(std::string_view sv) { + // Can be unsafe or intentional, like converting string_view into string + return std::string(sv); +} + +std::string explicitConstruction() { + // Cannot be std::string_view: returning address of local temporary object + // TODO: extract and return literal + return std::string("explicitConstruction"); +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
