[clang-tools-extra] r277677 - [clang-tidy] Inefficient string operation
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 8 16:38:13 PDT 2016
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 <dblaikie at gmail.com> wrote:
>
>
> On Wed, Aug 3, 2016 at 4:13 PM Alexander Kornienko via cfe-commits <
> cfe-commits at 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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160809/e7da057f/attachment-0001.html>
More information about the cfe-commits
mailing list