Index: clang-tidy/performance/CMakeLists.txt =================================================================== --- clang-tidy/performance/CMakeLists.txt +++ 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-tidy/performance/InefficientStringConcatenationCheck.h =================================================================== --- /dev/null +++ clang-tidy/performance/InefficientStringConcatenationCheck.h @@ -0,0 +1,38 @@ +//===--- 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-tidy/performance/InefficientStringConcatenationCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/performance/InefficientStringConcatenationCheck.cpp @@ -0,0 +1,97 @@ +//===--- 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 PlusOperatorMatcher = cxxBindTemporaryExpr(hasDescendant( + cxxOperatorCallExpr( + hasOverloadedOperatorName("+"), + hasAnyArgument(ignoringImpCasts(declRefExpr(BasicStringType))), + hasDescendant(BasicStringPlusOperator)) + .bind("plusOperator"))); + + const auto AssingOperator = cxxOperatorCallExpr( + hasOverloadedOperatorName("="), hasDescendant(BasicStringPlusOperator), + allOf(hasArgument( + 0, allOf(declRefExpr(BasicStringType), + declRefExpr(hasDeclaration(decl().bind("lhsStrT"))) + .bind("lhsStr"))), + hasArgument(1, stmt(hasDescendant(declRefExpr(hasDeclaration( + decl(equalsBoundNode("lhsStrT"))))))))); + + const auto DescendantAssignMatcher = hasDescendant(AssingOperator); + + if (!StrictMode) { + const auto SurroundLoopMatcher = + anyOf(hasAncestor(cxxForRangeStmt()), hasAncestor(whileStmt()), + hasAncestor(forStmt())); + + const auto WholeMatcher = + exprWithCleanups(SurroundLoopMatcher, DescendantAssignMatcher); + + Finder->addMatcher(WholeMatcher, this); + Finder->addMatcher(exprWithCleanups(SurroundLoopMatcher, + hasDescendant(PlusOperatorMatcher), + unless(WholeMatcher)), + this); + } else { + const auto WholeMatcher = exprWithCleanups(DescendantAssignMatcher); + Finder->addMatcher(WholeMatcher, this); + Finder->addMatcher(exprWithCleanups(hasDescendant(PlusOperatorMatcher), + unless(WholeMatcher)), + 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-tidy/performance/PerformanceTidyModule.cpp =================================================================== --- clang-tidy/performance/PerformanceTidyModule.cpp +++ clang-tidy/performance/PerformanceTidyModule.cpp @@ -14,6 +14,7 @@ #include "FasterStringFindCheck.h" #include "ForRangeCopyCheck.h" #include "ImplicitCastInLoopCheck.h" +#include "InefficientStringConcatenationCheck.h" #include "UnnecessaryCopyInitialization.h" #include "UnnecessaryValueParamCheck.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: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -184,6 +184,12 @@ Warns about range-based loop with a loop variable of const ref type where the type of the variable does not match the one returned by the iterator. +- New `performance-inefficient-string-concatenation + `_ check + + This check is to warn about the performance overhead arising from concatenating + strings, using the ``operator+``, instead of ``operator+=``. + - New `performance-unnecessary-value-param `_ check Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -102,6 +102,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: docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst @@ -0,0 +1,55 @@ +.. title:: clang-tidy - performance-inefficient-string-concatenation + +performance-inefficient-string-concatenation +============================================ + +The problem +----------- +This check is to warn 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: test/clang-tidy/performance-inefficient-string-concatenation.cpp =================================================================== --- /dev/null +++ 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; +}