Index: clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt @@ -4,6 +4,7 @@ FasterStringFindCheck.cpp ForRangeCopyCheck.cpp ImplicitCastInLoopCheck.cpp + InefficientStringConcatenationCheck.cpp PerformanceTidyModule.cpp UnnecessaryCopyInitialization.cpp UnnecessaryValueParamCheck.cpp Index: clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h +++ clang-tools-extra/trunk/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 Index: clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp +++ clang-tools-extra/trunk/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("lhsStr"); + const auto *PlusOperator = + Result.Nodes.getNodeAs("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 Index: clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp +++ clang-tools-extra/trunk/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 @@ "performance-for-range-copy"); CheckFactories.registerCheck( "performance-implicit-cast-in-loop"); + CheckFactories.registerCheck( + "performance-inefficient-string-concatenation"); CheckFactories.registerCheck( "performance-unnecessary-copy-initialization"); CheckFactories.registerCheck( Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/trunk/docs/ReleaseNotes.rst +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst @@ -69,6 +69,13 @@ Flags classes where some, but not all, special member functions are user-defined. +- New `performance-inefficient-string-concatenation + `_ check + + This check warns about the performance overhead arising from concatenating + strings using the ``operator+``, instead of ``operator+=``. + + Improvements to include-fixer ----------------------------- Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst @@ -114,6 +114,7 @@ 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 Index: clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst +++ clang-tools-extra/trunk/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)); + } Index: clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-string-concatenation.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-string-concatenation.cpp +++ clang-tools-extra/trunk/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 +class basic_string { +public: + basic_string() {} + ~basic_string() {} + basic_string *operator+=(const basic_string &) {} + friend basic_string operator+(const basic_string &, const basic_string &) {} +}; +typedef basic_string string; +typedef basic_string 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; +}