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/InefficientStringConcatenationCheck.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::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 > > Added: > clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.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
