[clang-tools-extra] r277677 - [clang-tidy] Inefficient string operation
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 8 08:35:21 PDT 2016
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/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 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/20160808/21458264/attachment-0001.html>
More information about the cfe-commits
mailing list