[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 {};
   }
 }
```

Author: irishrover

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

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>(

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

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

clang-tools-extra/docs/ReleaseNotes.rst

@@ -249,6 +249,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.
 

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"

clang-tools-extra/docs/clang-tidy/checks/performance/replace-with-string-view.rst

@@ -0,0 +1,68 @@
+.. 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`` along with their aliases and selects
+the proper kind of ``std::string_view`` to return.
+
+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.

clang-tools-extra/test/clang-tidy/checkers/performance/replace-with-string-view.cpp

@@ -0,0 +1,156 @@
+// 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) {
+  // Must not be converted to string_view because of use-after-free on stack
+  return s;
+}
+
+std::string returnIndirection(const char* ptr) {
+  // Can be unsafe or intentional, like converting string_view into string
+  return ptr;
+}
+
+std::string localBuffer() {
+  char buf[] = "local buffer";
+  // Must not be converted to string_view because of use-after-free on stack
+  return buf;
+}
+
+std::string returnConstVar() {
+  // TODO: seems safe
+  constexpr auto kVar = "const string";
+  return kVar;
+}
+
+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");
+}