[clang-tidy] Inefficient string operation

Patch by Bittner Barni!

Differential revision: https://reviews.llvm.org/D20196

llvm-svn: 277677

diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
index 32d8a24..71becce 100644 (file)
--- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
@@ -4,6 +4,7 @@ add_clang_library(clangTidyPerformanceModule
   FasterStringFindCheck.cpp
   ForRangeCopyCheck.cpp
   ImplicitCastInLoopCheck.cpp
+  InefficientStringConcatenationCheck.cpp
   PerformanceTidyModule.cpp
   UnnecessaryCopyInitialization.cpp
   UnnecessaryValueParamCheck.cpp

diff --git a/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp b/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
new file mode 100644 (file)
index 0000000..9e03fbc
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
@@ -0,0 +1,86 @@
+//===--- InefficientStringConcatenationCheck.cpp - clang-tidy--------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "InefficientStringConcatenationCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+void InefficientStringConcatenationCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "StrictMode", StrictMode);
+}
+
+InefficientStringConcatenationCheck::InefficientStringConcatenationCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context), StrictMode(Options.get("StrictMode", 0)) {}
+
+void InefficientStringConcatenationCheck::registerMatchers(
+    MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  const auto BasicStringType =
+      hasType(cxxRecordDecl(hasName("::std::basic_string")));
+
+  const auto BasicStringPlusOperator = cxxOperatorCallExpr(
+      hasOverloadedOperatorName("+"),
+      hasAnyArgument(ignoringImpCasts(declRefExpr(BasicStringType))));
+
+  const auto PlusOperator =
+      cxxOperatorCallExpr(
+          hasOverloadedOperatorName("+"),
+          hasAnyArgument(ignoringImpCasts(declRefExpr(BasicStringType))),
+          hasDescendant(BasicStringPlusOperator))
+          .bind("plusOperator");
+
+  const auto AssignOperator = cxxOperatorCallExpr(
+      hasOverloadedOperatorName("="),
+      hasArgument(0, declRefExpr(BasicStringType,
+                                 hasDeclaration(decl().bind("lhsStrT")))
+                         .bind("lhsStr")),
+      hasArgument(1, stmt(hasDescendant(declRefExpr(
+                         hasDeclaration(decl(equalsBoundNode("lhsStrT"))))))),
+      hasDescendant(BasicStringPlusOperator));
+
+  if (StrictMode) {
+    Finder->addMatcher(cxxOperatorCallExpr(anyOf(AssignOperator, PlusOperator)),
+                       this);
+  } else {
+    Finder->addMatcher(
+        cxxOperatorCallExpr(anyOf(AssignOperator, PlusOperator),
+                            hasAncestor(stmt(anyOf(cxxForRangeStmt(),
+                                                   whileStmt(), forStmt())))),
+        this);
+  }
+}
+
+void InefficientStringConcatenationCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *LhsStr = Result.Nodes.getNodeAs<DeclRefExpr>("lhsStr");
+  const auto *PlusOperator =
+      Result.Nodes.getNodeAs<CXXOperatorCallExpr>("plusOperator");
+  const auto DiagMsg =
+      "string concatenation results in allocation of unnecessary temporary "
+      "strings; consider using 'operator+=' or 'string::append()' instead";
+
+  if (LhsStr)
+    diag(LhsStr->getExprLoc(), DiagMsg);
+  else if (PlusOperator)
+    diag(PlusOperator->getExprLoc(), DiagMsg);
+}
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang

diff --git a/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.h b/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.h
new file mode 100644 (file)
index 0000000..12a154c
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.h
@@ -0,0 +1,41 @@
+//===--- InefficientStringConcatenationCheck.h - clang-tidy-----------*- C++
+//-*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTSTRINGCONCATENATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTSTRINGCONCATENATION_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+/// This check is to warn about the performance overhead arising from
+/// concatenating strings, using the operator+, instead of operator+=.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance-inefficient-string-concatenation.html
+class InefficientStringConcatenationCheck : public ClangTidyCheck {
+public:
+  InefficientStringConcatenationCheck(StringRef Name,
+                                      ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  const bool StrictMode;
+};
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTSTRINGCONCATENATION_H

diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
index 984a9fb..a722ffb 100644 (file)
--- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "InefficientStringConcatenationCheck.h"
 
 #include "FasterStringFindCheck.h"
 #include "ForRangeCopyCheck.h"
@@ -30,6 +31,8 @@ public:
         "performance-for-range-copy");
     CheckFactories.registerCheck<ImplicitCastInLoopCheck>(
         "performance-implicit-cast-in-loop");
+    CheckFactories.registerCheck<InefficientStringConcatenationCheck>(
+        "performance-inefficient-string-concatenation");
     CheckFactories.registerCheck<UnnecessaryCopyInitialization>(
         "performance-unnecessary-copy-initialization");
     CheckFactories.registerCheck<UnnecessaryValueParamCheck>(

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 03d1c0f..20ee2c3 100644 (file)
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -69,6 +69,13 @@ Improvements to clang-tidy
 
   Flags classes where some, but not all, special member functions are user-defined.
 
+- New `performance-inefficient-string-concatenation
+  <http://clang.llvm.org/extra/clang-tidy/checks/performance-inefficient-string-concatenation.html>`_ check
+
+  This check warns about the performance overhead arising from concatenating
+  strings using the ``operator+``, instead of ``operator+=``.
+
+
 Improvements to include-fixer
 -----------------------------
 

diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 6b73f3a..2b90545 100644 (file)
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -114,6 +114,7 @@ Clang-Tidy Checks
    performance-faster-string-find
    performance-for-range-copy
    performance-implicit-cast-in-loop
+   performance-inefficient-string-concatenation
    performance-unnecessary-copy-initialization
    performance-unnecessary-value-param
    readability-avoid-const-params-in-decls

diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst b/clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
new file mode 100644 (file)
index 0000000..a9169da
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
@@ -0,0 +1,49 @@
+.. title:: clang-tidy - performance-inefficient-string-concatenation
+
+performance-inefficient-string-concatenation
+============================================
+
+This check warns about the performance overhead arising from concatenating strings using the ``operator+``, for instance:
+    
+.. code:: c++
+
+    std::string a("Foo"), b("Bar");
+    a = a + b;
+
+Instead of this structure you should use ``operator+=`` or ``std::string``'s (``std::basic_string``) class member function ``append()``. For instance:
+   
+.. code:: c++
+
+   std::string a("Foo"), b("Baz");
+   for (int i = 0; i < 20000; ++i) {
+       a = a + "Bar" + b;
+   }
+
+Could be rewritten in a greatly more efficient way like:
+
+.. code:: c++
+
+   std::string a("Foo"), b("Baz");
+   for (int i = 0; i < 20000; ++i) {
+       a.append("Bar").append(b);
+   } 
+
+And this can be rewritten too:
+
+.. code:: c++
+
+   void f(const std::string&) {}
+   std::string a("Foo"), b("Baz");
+   void g() {
+       f(a + "Bar" + b);
+   }
+
+In a slightly more efficient way like:
+
+.. code:: c++
+
+   void f(const std::string&) {}
+   std::string a("Foo"), b("Baz");
+   void g() {
+       f(std::string(a).append("Bar").append(b));
+   }

diff --git a/clang-tools-extra/test/clang-tidy/performance-inefficient-string-concatenation.cpp b/clang-tools-extra/test/clang-tidy/performance-inefficient-string-concatenation.cpp
new file mode 100644 (file)
index 0000000..f4b293a
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/performance-inefficient-string-concatenation.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s performance-inefficient-string-concatenation %t
+
+namespace std {
+template <typename T>
+class basic_string {
+public:
+  basic_string() {}
+  ~basic_string() {}
+  basic_string<T> *operator+=(const basic_string<T> &) {}
+  friend basic_string<T> operator+(const basic_string<T> &, const basic_string<T> &) {}
+};
+typedef basic_string<char> string;
+typedef basic_string<wchar_t> wstring;
+}
+
+void f(std::string) {}
+std::string g(std::string) {}
+
+int main() {
+  std::string mystr1, mystr2;
+  std::wstring mywstr1, mywstr2;
+
+  for (int i = 0; i < 10; ++i) {
+    f(mystr1 + mystr2 + mystr1);
+    // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: string concatenation results in allocation of unnecessary temporary strings; consider using 'operator+=' or 'string::append()' instead
+    mystr1 = mystr1 + mystr2;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation
+    mystr1 = mystr2 + mystr2 + mystr2;
+    // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: string concatenation
+    mystr1 = mystr2 + mystr1;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation
+    mywstr1 = mywstr2 + mywstr1;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation
+    mywstr1 = mywstr2 + mywstr2 + mywstr2;
+    // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: string concatenation
+
+    mywstr1 = mywstr2 + mywstr2;
+    mystr1 = mystr2 + mystr2;
+    mystr1 += mystr2;
+    f(mystr2 + mystr1);
+    mystr1 = g(mystr1);
+  }
+  return 0;
+}