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( "performance-noexcept-swap"); + CheckFactories.registerCheck( + "performance-replace-with-string-view"); CheckFactories.registerCheck( "performance-trivially-destructible"); CheckFactories.registerCheck( 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("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 + ` 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 ` 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 `, "Yes" :doc:`performance-noexcept-move-constructor `, "Yes" :doc:`performance-noexcept-swap `, "Yes" + :doc:`performance-replace-with-string-view `, "Yes" :doc:`performance-trivially-destructible `, "Yes" :doc:`performance-type-promotion-in-math-fn `, "Yes" :doc:`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 + class basic_string_view { + public: + basic_string_view(const CharT *); + basic_string_view(); + }; + using string_view = basic_string_view; + using wstring_view = basic_string_view; + using u16string_view = basic_string_view; + + template + class basic_string { + public: + basic_string(); + basic_string(const CharT *); + basic_string(basic_string_view); + + basic_string operator+(const basic_string &) const; + }; + using string = basic_string; + using wstring = basic_string; + using u16string = basic_string; +} + +// ========================================================== +// 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"); +} +