<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Aug 3, 2016 at 4:13 PM Alexander Kornienko via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: alexfh<br>
Date: Wed Aug 3 18:06:03 2016<br>
New Revision: 277677<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=277677&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=277677&view=rev</a><br>
Log:<br>
[clang-tidy] Inefficient string operation<br></blockquote><div><br></div><div>A more detailed commit message might be nice. The code review included some information that might've been a good start:<br><br>"<span style="font-family:"segoe ui","segoe ui web regular","segoe ui symbol","helvetica neue",helvetica,arial,sans-serif;font-size:13px;line-height:18.85px">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."</span></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Patch by Bittner Barni!<br>
<br>
Differential revision: <a href="https://reviews.llvm.org/D20196" rel="noreferrer" target="_blank">https://reviews.llvm.org/D20196</a><br>
<br>
Added:<br>
clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp<br>
clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h<br>
clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst<br>
clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-string-concatenation.cpp<br>
Modified:<br>
clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt<br>
clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp<br>
clang-tools-extra/trunk/docs/ReleaseNotes.rst<br>
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst<br>
<br>
Modified: clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt?rev=277677&r1=277676&r2=277677&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt?rev=277677&r1=277676&r2=277677&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt (original)<br>
+++ clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt Wed Aug 3 18:06:03 2016<br>
@@ -4,6 +4,7 @@ add_clang_library(clangTidyPerformanceMo<br>
FasterStringFindCheck.cpp<br>
ForRangeCopyCheck.cpp<br>
ImplicitCastInLoopCheck.cpp<br>
+ InefficientStringConcatenationCheck.cpp<br>
PerformanceTidyModule.cpp<br>
UnnecessaryCopyInitialization.cpp<br>
UnnecessaryValueParamCheck.cpp<br>
<br>
Added: clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp?rev=277677&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp?rev=277677&view=auto</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp (added)<br>
+++ clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp Wed Aug 3 18:06:03 2016<br>
@@ -0,0 +1,86 @@<br>
+//===--- InefficientStringConcatenationCheck.cpp - clang-tidy--------------===//<br>
+//<br>
+// The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#include "InefficientStringConcatenationCheck.h"<br>
+#include "clang/AST/ASTContext.h"<br>
+#include "clang/ASTMatchers/ASTMatchFinder.h"<br>
+<br>
+using namespace clang::ast_matchers;<br>
+<br>
+namespace clang {<br>
+namespace tidy {<br>
+namespace performance {<br>
+<br>
+void InefficientStringConcatenationCheck::storeOptions(<br>
+ ClangTidyOptions::OptionMap &Opts) {<br>
+ Options.store(Opts, "StrictMode", StrictMode);<br>
+}<br>
+<br>
+InefficientStringConcatenationCheck::InefficientStringConcatenationCheck(<br>
+ StringRef Name, ClangTidyContext *Context)<br>
+ : ClangTidyCheck(Name, Context), StrictMode(Options.get("StrictMode", 0)) {}<br>
+<br>
+void InefficientStringConcatenationCheck::registerMatchers(<br>
+ MatchFinder *Finder) {<br>
+ if (!getLangOpts().CPlusPlus)<br>
+ return;<br>
+<br>
+ const auto BasicStringType =<br>
+ hasType(cxxRecordDecl(hasName("::std::basic_string")));<br>
+<br>
+ const auto BasicStringPlusOperator = cxxOperatorCallExpr(<br>
+ hasOverloadedOperatorName("+"),<br>
+ hasAnyArgument(ignoringImpCasts(declRefExpr(BasicStringType))));<br>
+<br>
+ const auto PlusOperator =<br>
+ cxxOperatorCallExpr(<br>
+ hasOverloadedOperatorName("+"),<br>
+ hasAnyArgument(ignoringImpCasts(declRefExpr(BasicStringType))),<br>
+ hasDescendant(BasicStringPlusOperator))<br>
+ .bind("plusOperator");<br>
+<br>
+ const auto AssignOperator = cxxOperatorCallExpr(<br>
+ hasOverloadedOperatorName("="),<br>
+ hasArgument(0, declRefExpr(BasicStringType,<br>
+ hasDeclaration(decl().bind("lhsStrT")))<br>
+ .bind("lhsStr")),<br>
+ hasArgument(1, stmt(hasDescendant(declRefExpr(<br>
+ hasDeclaration(decl(equalsBoundNode("lhsStrT"))))))),<br>
+ hasDescendant(BasicStringPlusOperator));<br>
+<br>
+ if (StrictMode) {<br>
+ Finder->addMatcher(cxxOperatorCallExpr(anyOf(AssignOperator, PlusOperator)),<br>
+ this);<br>
+ } else {<br>
+ Finder->addMatcher(<br>
+ cxxOperatorCallExpr(anyOf(AssignOperator, PlusOperator),<br>
+ hasAncestor(stmt(anyOf(cxxForRangeStmt(),<br>
+ whileStmt(), forStmt())))),<br>
+ this);<br>
+ }<br>
+}<br>
+<br>
+void InefficientStringConcatenationCheck::check(<br>
+ const MatchFinder::MatchResult &Result) {<br>
+ const auto *LhsStr = Result.Nodes.getNodeAs<DeclRefExpr>("lhsStr");<br>
+ const auto *PlusOperator =<br>
+ Result.Nodes.getNodeAs<CXXOperatorCallExpr>("plusOperator");<br>
+ const auto DiagMsg =<br>
+ "string concatenation results in allocation of unnecessary temporary "<br>
+ "strings; consider using 'operator+=' or 'string::append()' instead";<br>
+<br>
+ if (LhsStr)<br>
+ diag(LhsStr->getExprLoc(), DiagMsg);<br>
+ else if (PlusOperator)<br>
+ diag(PlusOperator->getExprLoc(), DiagMsg);<br>
+}<br>
+<br>
+} // namespace performance<br>
+} // namespace tidy<br>
+} // namespace clang<br>
<br>
Added: clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h?rev=277677&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h?rev=277677&view=auto</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h (added)<br>
+++ clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h Wed Aug 3 18:06:03 2016<br>
@@ -0,0 +1,41 @@<br>
+//===--- InefficientStringConcatenationCheck.h - clang-tidy-----------*- C++<br>
+//-*-===//<br>
+//<br>
+// The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTSTRINGCONCATENATION_H<br>
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTSTRINGCONCATENATION_H<br>
+<br>
+#include "../ClangTidy.h"<br>
+<br>
+namespace clang {<br>
+namespace tidy {<br>
+namespace performance {<br>
+<br>
+/// This check is to warn about the performance overhead arising from<br>
+/// concatenating strings, using the operator+, instead of operator+=.<br>
+///<br>
+/// For the user-facing documentation see:<br>
+/// <a href="http://clang.llvm.org/extra/clang-tidy/checks/performance-inefficient-string-concatenation.html" rel="noreferrer" target="_blank">http://clang.llvm.org/extra/clang-tidy/checks/performance-inefficient-string-concatenation.html</a><br>
+class InefficientStringConcatenationCheck : public ClangTidyCheck {<br>
+public:<br>
+ InefficientStringConcatenationCheck(StringRef Name,<br>
+ ClangTidyContext *Context);<br>
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;<br>
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;<br>
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;<br>
+<br>
+private:<br>
+ const bool StrictMode;<br>
+};<br>
+<br>
+} // namespace performance<br>
+} // namespace tidy<br>
+} // namespace clang<br>
+<br>
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTSTRINGCONCATENATION_H<br>
<br>
Modified: clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp?rev=277677&r1=277676&r2=277677&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp?rev=277677&r1=277676&r2=277677&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp (original)<br>
+++ clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp Wed Aug 3 18:06:03 2016<br>
@@ -10,6 +10,7 @@<br>
#include "../ClangTidy.h"<br>
#include "../ClangTidyModule.h"<br>
#include "../ClangTidyModuleRegistry.h"<br>
+#include "InefficientStringConcatenationCheck.h"<br>
<br>
#include "FasterStringFindCheck.h"<br>
#include "ForRangeCopyCheck.h"<br>
@@ -30,6 +31,8 @@ public:<br>
"performance-for-range-copy");<br>
CheckFactories.registerCheck<ImplicitCastInLoopCheck>(<br>
"performance-implicit-cast-in-loop");<br>
+ CheckFactories.registerCheck<InefficientStringConcatenationCheck>(<br>
+ "performance-inefficient-string-concatenation");<br>
CheckFactories.registerCheck<UnnecessaryCopyInitialization>(<br>
"performance-unnecessary-copy-initialization");<br>
CheckFactories.registerCheck<UnnecessaryValueParamCheck>(<br>
<br>
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=277677&r1=277676&r2=277677&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=277677&r1=277676&r2=277677&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)<br>
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed Aug 3 18:06:03 2016<br>
@@ -69,6 +69,13 @@ Improvements to clang-tidy<br>
<br>
Flags classes where some, but not all, special member functions are user-defined.<br>
<br>
+- New `performance-inefficient-string-concatenation<br>
+ <<a href="http://clang.llvm.org/extra/clang-tidy/checks/performance-inefficient-string-concatenation.html" rel="noreferrer" target="_blank">http://clang.llvm.org/extra/clang-tidy/checks/performance-inefficient-string-concatenation.html</a>>`_ check<br>
+<br>
+ This check warns about the performance overhead arising from concatenating<br>
+ strings using the ``operator+``, instead of ``operator+=``.<br>
+<br>
+<br>
Improvements to include-fixer<br>
-----------------------------<br>
<br>
<br>
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=277677&r1=277676&r2=277677&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=277677&r1=277676&r2=277677&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)<br>
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Wed Aug 3 18:06:03 2016<br>
@@ -114,6 +114,7 @@ Clang-Tidy Checks<br>
performance-faster-string-find<br>
performance-for-range-copy<br>
performance-implicit-cast-in-loop<br>
+ performance-inefficient-string-concatenation<br>
performance-unnecessary-copy-initialization<br>
performance-unnecessary-value-param<br>
readability-avoid-const-params-in-decls<br>
<br>
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst?rev=277677&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst?rev=277677&view=auto</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst (added)<br>
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst Wed Aug 3 18:06:03 2016<br>
@@ -0,0 +1,49 @@<br>
+.. title:: clang-tidy - performance-inefficient-string-concatenation<br>
+<br>
+performance-inefficient-string-concatenation<br>
+============================================<br>
+<br>
+This check warns about the performance overhead arising from concatenating strings using the ``operator+``, for instance:<br>
+<br>
+.. code:: c++<br>
+<br>
+ std::string a("Foo"), b("Bar");<br>
+ a = a + b;<br>
+<br>
+Instead of this structure you should use ``operator+=`` or ``std::string``'s (``std::basic_string``) class member function ``append()``. For instance:<br>
+<br>
+.. code:: c++<br>
+<br>
+ std::string a("Foo"), b("Baz");<br>
+ for (int i = 0; i < 20000; ++i) {<br>
+ a = a + "Bar" + b;<br>
+ }<br>
+<br>
+Could be rewritten in a greatly more efficient way like:<br>
+<br>
+.. code:: c++<br>
+<br>
+ std::string a("Foo"), b("Baz");<br>
+ for (int i = 0; i < 20000; ++i) {<br>
+ a.append("Bar").append(b);<br>
+ }<br>
+<br>
+And this can be rewritten too:<br>
+<br>
+.. code:: c++<br>
+<br>
+ void f(const std::string&) {}<br>
+ std::string a("Foo"), b("Baz");<br>
+ void g() {<br>
+ f(a + "Bar" + b);<br>
+ }<br>
+<br>
+In a slightly more efficient way like:<br>
+<br>
+.. code:: c++<br>
+<br>
+ void f(const std::string&) {}<br>
+ std::string a("Foo"), b("Baz");<br>
+ void g() {<br>
+ f(std::string(a).append("Bar").append(b));<br>
+ }<br>
<br>
Added: clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-string-concatenation.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-string-concatenation.cpp?rev=277677&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-string-concatenation.cpp?rev=277677&view=auto</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-string-concatenation.cpp (added)<br>
+++ clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-string-concatenation.cpp Wed Aug 3 18:06:03 2016<br>
@@ -0,0 +1,44 @@<br>
+// RUN: %check_clang_tidy %s performance-inefficient-string-concatenation %t<br>
+<br>
+namespace std {<br>
+template <typename T><br>
+class basic_string {<br>
+public:<br>
+ basic_string() {}<br>
+ ~basic_string() {}<br>
+ basic_string<T> *operator+=(const basic_string<T> &) {}<br>
+ friend basic_string<T> operator+(const basic_string<T> &, const basic_string<T> &) {}<br>
+};<br>
+typedef basic_string<char> string;<br>
+typedef basic_string<wchar_t> wstring;<br>
+}<br>
+<br>
+void f(std::string) {}<br>
+std::string g(std::string) {}<br>
+<br>
+int main() {<br>
+ std::string mystr1, mystr2;<br>
+ std::wstring mywstr1, mywstr2;<br>
+<br>
+ for (int i = 0; i < 10; ++i) {<br>
+ f(mystr1 + mystr2 + mystr1);<br>
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: string concatenation results in allocation of unnecessary temporary strings; consider using 'operator+=' or 'string::append()' instead<br>
+ mystr1 = mystr1 + mystr2;<br>
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation<br>
+ mystr1 = mystr2 + mystr2 + mystr2;<br>
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: string concatenation<br>
+ mystr1 = mystr2 + mystr1;<br>
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation<br>
+ mywstr1 = mywstr2 + mywstr1;<br>
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation<br>
+ mywstr1 = mywstr2 + mywstr2 + mywstr2;<br>
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: string concatenation<br>
+<br>
+ mywstr1 = mywstr2 + mywstr2;<br>
+ mystr1 = mystr2 + mystr2;<br>
+ mystr1 += mystr2;<br>
+ f(mystr2 + mystr1);<br>
+ mystr1 = g(mystr1);<br>
+ }<br>
+ return 0;<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div>