https://github.com/irishrover updated https://github.com/llvm/llvm-project/pull/172170
>From 7f9424ab7955031ca3ee343dafa27b4186f5bd48 Mon Sep 17 00:00:00 2001 From: Zinovy Nis <[email protected]> Date: Sat, 13 Dec 2025 13:47:01 +0300 Subject: [PATCH 1/3] [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: 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 {}; } } --- .../clang-tidy/performance/CMakeLists.txt | 1 + .../performance/PerformanceTidyModule.cpp | 3 + .../ReplaceWithStringViewCheck.cpp | 74 +++++++++ .../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 | 142 ++++++++++++++++++ 8 files changed, 334 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..0cb11cda8e1aa --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/ReplaceWithStringViewCheck.cpp @@ -0,0 +1,74 @@ +//===----------------------------------------------------------------------===// +// +// 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 { + +namespace { +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"; +} +} // namespace + +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 27467d082ea1e..03470b8c78489 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. + + Detects functions returning std::[w|u8|u16|u32]string where + return type can be changed to std::[...]string_view. + - 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..a5dc48b450c40 --- /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. + +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::u16string 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..625d729a6e4c9 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/replace-with-string-view.cpp @@ -0,0 +1,142 @@ +// 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"; + 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) { + // TODO: should be convertible to std::string_view + return ptr; +} + +std::string passStringView(std::string_view sv) { + // TODO: should be convertible to std::string_view + return std::string(sv); +} + +std::string explicitConstruction() { + // Cannot be std::string_view: returning address of local temporary object + return std::string("explicitConstruction"); +} + >From ef89435e54560cea524fb7c046f37eb2051b3a4e Mon Sep 17 00:00:00 2001 From: Zinovy Nis <[email protected]> Date: Sun, 14 Dec 2025 10:41:51 +0300 Subject: [PATCH 2/3] Update clang-tools-extra/docs/ReleaseNotes.rst Co-authored-by: EugeneZelenko <[email protected]> --- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 03470b8c78489..0d8b6532f9358 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -247,8 +247,8 @@ New checks - New :doc:`performance-replace-with-string-view <clang-tidy/checks/performance/replace-with-string-view>` check. - Detects functions returning std::[w|u8|u16|u32]string where - return type can be changed to std::[...]string_view. + Detects functions returning ``std::[w|u8|u16|u32]string`` where + return type can be changed to ``std::[...]string_view``. - New :doc:`readability-redundant-parentheses <clang-tidy/checks/readability/redundant-parentheses>` check. >From eac66f31594f622392a0c8852ee7794e0d9a076e Mon Sep 17 00:00:00 2001 From: Zinovy Nis <[email protected]> Date: Sun, 14 Dec 2025 10:41:59 +0300 Subject: [PATCH 3/3] Update clang-tools-extra/docs/clang-tidy/checks/performance/replace-with-string-view.rst Co-authored-by: EugeneZelenko <[email protected]> --- .../clang-tidy/checks/performance/replace-with-string-view.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index a5dc48b450c40..03623ae8fc278 100644 --- 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 @@ -13,7 +13,7 @@ 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 +such allocations can be eliminated sometimes by using ``std::string_view`` as a return type. This check looks for such functions returning ``std::string`` _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
