Agreed. Normally, arc copies the full patch description from Phab, but this time the patch didn't apply cleanly, so I had to copy the commit message by hand and missed a substantial part of it.
On Mon, Aug 8, 2016 at 5:35 PM, David Blaikie <dblai...@gmail.com> wrote: > > > On Wed, Aug 3, 2016 at 4:13 PM Alexander Kornienko via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: alexfh >> Date: Wed Aug 3 18:06:03 2016 >> New Revision: 277677 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=277677&view=rev >> Log: >> [clang-tidy] Inefficient string operation >> > > A more detailed commit message might be nice. The code review included > some information that might've been a good start: > > "This checker is to warn about the performance overhead caused by > concatenating strings using the operator+ instead of basic_string's append > or operator+=. It is configurable. In strict mode it matches every instance > of a supposed inefficient concatenation, in non-strict mode it matches only > those which are inside a loop." > > >> >> Patch by Bittner Barni! >> >> Differential revision: https://reviews.llvm.org/D20196 >> >> Added: >> clang-tools-extra/trunk/clang-tidy/performance/ >> InefficientStringConcatenationCheck.cpp >> clang-tools-extra/trunk/clang-tidy/performance/ >> InefficientStringConcatenationCheck.h >> clang-tools-extra/trunk/docs/clang-tidy/checks/performance- >> inefficient-string-concatenation.rst >> clang-tools-extra/trunk/test/clang-tidy/performance- >> inefficient-string-concatenation.cpp >> Modified: >> clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt >> clang-tools-extra/trunk/clang-tidy/performance/ >> PerformanceTidyModule.cpp >> clang-tools-extra/trunk/docs/ReleaseNotes.rst >> clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst >> >> Modified: clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt >> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/ >> trunk/clang-tidy/performance/CMakeLists.txt?rev=277677&r1= >> 277676&r2=277677&view=diff >> ============================================================ >> ================== >> --- clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt >> (original) >> +++ clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt Wed >> Aug 3 18:06:03 2016 >> @@ -4,6 +4,7 @@ add_clang_library(clangTidyPerformanceMo >> FasterStringFindCheck.cpp >> ForRangeCopyCheck.cpp >> ImplicitCastInLoopCheck.cpp >> + InefficientStringConcatenationCheck.cpp >> PerformanceTidyModule.cpp >> UnnecessaryCopyInitialization.cpp >> UnnecessaryValueParamCheck.cpp >> >> Added: clang-tools-extra/trunk/clang-tidy/performance/ >> InefficientStringConcatenationCheck.cpp >> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/ >> trunk/clang-tidy/performance/InefficientStringConcatenation >> Check.cpp?rev=277677&view=auto >> ============================================================ >> ================== >> --- clang-tools-extra/trunk/clang-tidy/performance/ >> InefficientStringConcatenationCheck.cpp (added) >> +++ clang-tools-extra/trunk/clang-tidy/performance/ >> InefficientStringConcatenationCheck.cpp Wed Aug 3 18:06:03 2016 >> @@ -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::InefficientStringConcatenation >> Check( >> + 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 >> >> Added: clang-tools-extra/trunk/clang-tidy/performance/ >> InefficientStringConcatenationCheck.h >> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/ >> trunk/clang-tidy/performance/InefficientStringConcatenation >> Check.h?rev=277677&view=auto >> ============================================================ >> ================== >> --- clang-tools-extra/trunk/clang-tidy/performance/ >> InefficientStringConcatenationCheck.h (added) >> +++ clang-tools-extra/trunk/clang-tidy/performance/ >> InefficientStringConcatenationCheck.h Wed Aug 3 18:06:03 2016 >> @@ -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 >> >> Modified: clang-tools-extra/trunk/clang-tidy/performance/ >> PerformanceTidyModule.cpp >> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/ >> trunk/clang-tidy/performance/PerformanceTidyModule.cpp?rev= >> 277677&r1=277676&r2=277677&view=diff >> ============================================================ >> ================== >> --- clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp >> (original) >> +++ clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp >> Wed Aug 3 18:06:03 2016 >> @@ -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>( >> >> Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst >> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/ >> trunk/docs/ReleaseNotes.rst?rev=277677&r1=277676&r2=277677&view=diff >> ============================================================ >> ================== >> --- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original) >> +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed Aug 3 18:06:03 >> 2016 >> @@ -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 >> ----------------------------- >> >> >> Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst >> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/ >> trunk/docs/clang-tidy/checks/list.rst?rev=277677&r1=277676& >> r2=277677&view=diff >> ============================================================ >> ================== >> --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original) >> +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Wed Aug 3 >> 18:06:03 2016 >> @@ -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 >> >> Added: clang-tools-extra/trunk/docs/clang-tidy/checks/performance- >> inefficient-string-concatenation.rst >> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/ >> trunk/docs/clang-tidy/checks/performance-inefficient- >> string-concatenation.rst?rev=277677&view=auto >> ============================================================ >> ================== >> --- clang-tools-extra/trunk/docs/clang-tidy/checks/performance- >> inefficient-string-concatenation.rst (added) >> +++ clang-tools-extra/trunk/docs/clang-tidy/checks/performance- >> inefficient-string-concatenation.rst Wed Aug 3 18:06:03 2016 >> @@ -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)); >> + } >> >> Added: clang-tools-extra/trunk/test/clang-tidy/performance- >> inefficient-string-concatenation.cpp >> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/ >> trunk/test/clang-tidy/performance-inefficient- >> string-concatenation.cpp?rev=277677&view=auto >> ============================================================ >> ================== >> --- clang-tools-extra/trunk/test/clang-tidy/performance- >> inefficient-string-concatenation.cpp (added) >> +++ clang-tools-extra/trunk/test/clang-tidy/performance- >> inefficient-string-concatenation.cpp Wed Aug 3 18:06:03 2016 >> @@ -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; >> +} >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
