Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
Eugene.Zelenko added a comment. Please see PR28836. In some cases check should recommend to use insert(). Repository: rL LLVM https://reviews.llvm.org/D20196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
This revision was automatically updated to reflect the committed changes.
Closed by commit rL277677: [clang-tidy] Inefficient string operation (authored
by alexfh).
Changed prior to commit:
https://reviews.llvm.org/D20196?vs=66492&id=66730#toc
Repository:
rL LLVM
https://reviews.llvm.org/D20196
Files:
clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h
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
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
Index: clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h
+++ clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h
@@ -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
Index: clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp
@@ -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 @@
"performance-for-range-copy");
CheckFactories.registerCheck(
"performance-implicit-cast-in-loop");
+CheckFactories.registerCheck(
+"performance-inefficient-string-concatenation");
CheckFactories.registerCheck(
"performance-unnecessary-copy-initialization");
CheckFactories.registerCheck(
Index: clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
@@ -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), StrictM
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
Prazek added a comment. In https://reviews.llvm.org/D20196#504397, @bittnerbarni wrote: > I'm planning to submit more patches in the future, as I have time for them. > So it wouldn't be in vain :) http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access https://reviews.llvm.org/D20196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni added a comment. I'm planning to submit more patches in the future, as I have time for them. So it wouldn't be in vain :) https://reviews.llvm.org/D20196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
Prazek added a subscriber: Prazek. Prazek added a comment. In https://reviews.llvm.org/D20196#504394, @bittnerbarni wrote: > Thank you for all the assistance. Could you please do that? btw obtaining commit access and commiting is very simple, so if you are planning to send us some more cool patches then I think you should get the commit access :) https://reviews.llvm.org/D20196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni added a comment. Thank you for all the assistance. Could you please do that? https://reviews.llvm.org/D20196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good. Thank you for working on this! Do you need me to commit the patch for you? https://reviews.llvm.org/D20196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni marked 14 inline comments as done. bittnerbarni added a comment. https://reviews.llvm.org/D20196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni updated this revision to Diff 66492.
https://reviews.llvm.org/D20196
Files:
clang-tidy/performance/CMakeLists.txt
clang-tidy/performance/InefficientStringConcatenationCheck.cpp
clang-tidy/performance/InefficientStringConcatenationCheck.h
clang-tidy/performance/PerformanceTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
test/clang-tidy/performance-inefficient-string-concatenation.cpp
Index: test/clang-tidy/performance-inefficient-string-concatenation.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-inefficient-string-concatenation.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s performance-inefficient-string-concatenation %t
+
+namespace std {
+template
+class basic_string {
+public:
+ basic_string() {}
+ ~basic_string() {}
+ basic_string *operator+=(const basic_string &) {}
+ friend basic_string operator+(const basic_string &, const basic_string &) {}
+};
+typedef basic_string string;
+typedef basic_string 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;
+}
Index: docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
@@ -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 < 2; ++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 < 2; ++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));
+ }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -102,6 +102,7 @@
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
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -184,6 +184,12 @@
Warns about range-based loop with a loop variable of const ref type where the
type of the variable does not match the one returned by the iterator.
+- New `performance-inefficient-string-concatenation
+ `_ check
+
+ This check is to warn about the performance overhead arising from concatenating
+ strings, using the ``operator+``, instead of ``operator+=``.
+
- New `performance-unnecessary-value-param
`_ check
Index: clang-tidy/performance/Performa
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Sorry for the delay. I almost missed that there was an update to the patch. Please mark addressed comments as "Done". This way it's easier for me to track changes and it's easier for you not to miss review comments. One more thing that this check lacks is fix-it hints, however, this can be added later. Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.h:26 @@ +25,3 @@ + public: + InefficientStringConcatenationCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; The formatting is still off (specifically, indentation of `public:` and `private:` and the lack of an empty line before `private:`). https://reviews.llvm.org/D20196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni updated this revision to Diff 65606.
https://reviews.llvm.org/D20196
Files:
clang-tidy/performance/CMakeLists.txt
clang-tidy/performance/InefficientStringConcatenationCheck.cpp
clang-tidy/performance/InefficientStringConcatenationCheck.h
clang-tidy/performance/PerformanceTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
test/clang-tidy/performance-inefficient-string-concatenation.cpp
Index: test/clang-tidy/performance-inefficient-string-concatenation.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-inefficient-string-concatenation.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s performance-inefficient-string-concatenation %t
+
+namespace std {
+template
+class basic_string {
+public:
+ basic_string() {}
+ ~basic_string() {}
+ basic_string *operator+=(const basic_string &) {}
+ friend basic_string operator+(const basic_string &, const basic_string &) {}
+};
+typedef basic_string string;
+typedef basic_string 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;
+}
Index: docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
@@ -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 < 2; ++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 < 2; ++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));
+ }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -102,6 +102,7 @@
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
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -184,6 +184,12 @@
Warns about range-based loop with a loop variable of const ref type where the
type of the variable does not match the one returned by the iterator.
+- New `performance-inefficient-string-concatenation
+ `_ check
+
+ This check is to warn about the performance overhead arising from concatenating
+ strings, using the ``operator+``, instead of ``operator+=``.
+
- New `performance-unnecessary-value-param
`_ check
Index: clang-tidy/performance/Performa
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
alexfh added a comment.
A couple of nits.
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:51
@@ +50,3 @@
+ hasArgument(
+ 0, declRefExpr(BasicStringType),
+ declRefExpr(hasDeclaration(decl().bind("lhsStrT"))).bind("lhsStr")),
As noted earlier, there's no need to repeat `declRefExpr`. This should be:
declRefExpr(BasicStringType, hasDeclaration(decl().bind("lhsStrT")))
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.h:25
@@ +24,3 @@
+class InefficientStringConcatenationCheck : public ClangTidyCheck {
+ public:
+ InefficientStringConcatenationCheck(StringRef Name, ClangTidyContext
*Context);
Please format the file with `clang-format -style=llvm`.
https://reviews.llvm.org/D20196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni added inline comments.
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:55
@@ +54,3 @@
+ hasDeclaration(decl(equalsBoundNode("lhsStrT"))),
+ hasDescendant(BasicStringPlusOperator));
+
Yes this is the result of clang-format.
https://reviews.llvm.org/D20196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni updated this revision to Diff 65384.
bittnerbarni marked an inline comment as done.
https://reviews.llvm.org/D20196
Files:
clang-tidy/performance/CMakeLists.txt
clang-tidy/performance/InefficientStringConcatenationCheck.cpp
clang-tidy/performance/InefficientStringConcatenationCheck.h
clang-tidy/performance/PerformanceTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
test/clang-tidy/performance-inefficient-string-concatenation.cpp
Index: test/clang-tidy/performance-inefficient-string-concatenation.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-inefficient-string-concatenation.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s performance-inefficient-string-concatenation %t
+
+namespace std {
+template
+class basic_string {
+public:
+ basic_string() {}
+ ~basic_string() {}
+ basic_string *operator+=(const basic_string &) {}
+ friend basic_string operator+(const basic_string &, const basic_string &) {}
+};
+typedef basic_string string;
+typedef basic_string 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;
+}
Index: docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
@@ -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 < 2; ++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 < 2; ++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));
+ }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -102,6 +102,7 @@
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
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -184,6 +184,12 @@
Warns about range-based loop with a loop variable of const ref type where the
type of the variable does not match the one returned by the iterator.
+- New `performance-inefficient-string-concatenation
+ `_ check
+
+ This check is to warn about the performance overhead arising from concatenating
+ strings, using the ``operator+``, instead of ``operator+=``.
+
- New `performance-unnecessary-value-param
`_
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
alexfh added a comment.
A few more nits.
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:50
@@ +49,3 @@
+ hasOverloadedOperatorName("="),
+ hasArgument(0, allOf(declRefExpr(BasicStringType),
+ declRefExpr(hasDeclaration(decl().bind("lhsStrT")))
The `allOf(declRefExpr(x), declRefExpr(y))` construct can be replaced with
`declRefExpr(x, y)`.
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:54
@@ +53,3 @@
+ hasArgument(1, stmt(hasDescendant(declRefExpr(
+ hasDeclaration(decl(equalsBoundNode("lhsStrT"))),
+ hasDescendant(BasicStringPlusOperator));
Indentation is confusing - as though `hasDeclaration` is an argument of `stmt`.
Is it a result of clang-format?
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:63
@@ +62,3 @@
+cxxOperatorCallExpr(
+hasAncestor(stmt(anyOf(cxxForRangeStmt(), whileStmt(),
forStmt(,
+anyOf(AssignOperator, PlusOperator)),
`hasAncestor` is potentially more expensive, so it should go last.
https://reviews.llvm.org/D20196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni updated this revision to Diff 63098.
bittnerbarni added a comment.
Thank you, for your valuable comments Alexander!
http://reviews.llvm.org/D20196
Files:
clang-tidy/performance/CMakeLists.txt
clang-tidy/performance/InefficientStringConcatenationCheck.cpp
clang-tidy/performance/InefficientStringConcatenationCheck.h
clang-tidy/performance/PerformanceTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
test/clang-tidy/performance-inefficient-string-concatenation.cpp
Index: test/clang-tidy/performance-inefficient-string-concatenation.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-inefficient-string-concatenation.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s performance-inefficient-string-concatenation %t
+
+namespace std {
+template
+class basic_string {
+public:
+ basic_string() {}
+ ~basic_string() {}
+ basic_string *operator+=(const basic_string &) {}
+ friend basic_string operator+(const basic_string &, const basic_string &) {}
+};
+typedef basic_string string;
+typedef basic_string 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;
+}
Index: docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
@@ -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 < 2; ++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 < 2; ++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));
+ }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -102,6 +102,7 @@
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
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -184,6 +184,12 @@
Warns about range-based loop with a loop variable of const ref type where the
type of the variable does not match the one returned by the iterator.
+- New `performance-inefficient-string-concatenation
+ `_ check
+
+ This check is to warn about the performance overhead arising from concatenating
+ strings, using the ``operator+``, instead of ``operator+=``.
+
- New `performance-unnecessary-value-param
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
alexfh added inline comments.
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:50
@@ +49,3 @@
+ hasOverloadedOperatorName("="), hasDescendant(BasicStringPlusOperator),
+ allOf(hasArgument(
+0, allOf(declRefExpr(BasicStringType),
1. No need for `allOf`, node matchers are implicitly "all of".
2. `hasDescendant()` is potentially the most expensive part here and should go
after all other constraints.
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:68
@@ +67,3 @@
+this);
+ }
+}
For 1: if `hasAncestor(anyOf(cxxForRangeStmt(), ...))` doesn't work, you can
try `hasAncestor(stmt(anyOf(cxxForRangeStmt(), ...))`. In any case, repeated
`hasAncestor` matchers don't make the check faster ;)
For 2: `exprWithCleanups` and other implicit nodes can be skipped using the
`ignoringImplicit()` matchers. Leave `hasAncestor` for the cases, where there
can actually be arbitrary nodes in the path.
Here you're matching some arbitrary intermediate node (`exprWithCleanups`) and
then you're constraining its parents and its children. This doesn't make much
sense and it's rather inefficient. Since you're interested in the operators in
the first place, try rearranging the matcher like this:
```
cxxOperatorCallExpr(
anyOf(AssignOperator, PlusOperator),
hasAncestor(stmt(anyOf(cxxForRangeStmt(), whileStmt(), forStmt()
```
http://reviews.llvm.org/D20196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni updated this revision to Diff 61893.
http://reviews.llvm.org/D20196
Files:
clang-tidy/performance/CMakeLists.txt
clang-tidy/performance/InefficientStringConcatenationCheck.cpp
clang-tidy/performance/InefficientStringConcatenationCheck.h
clang-tidy/performance/PerformanceTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
test/clang-tidy/performance-inefficient-string-concatenation.cpp
Index: test/clang-tidy/performance-inefficient-string-concatenation.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-inefficient-string-concatenation.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s performance-inefficient-string-concatenation %t
+
+namespace std {
+template
+class basic_string {
+public:
+ basic_string() {}
+ ~basic_string() {}
+ basic_string *operator+=(const basic_string &) {}
+ friend basic_string operator+(const basic_string &, const basic_string &) {}
+};
+typedef basic_string string;
+typedef basic_string 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;
+}
Index: docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
@@ -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 < 2; ++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 < 2; ++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));
+ }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -102,6 +102,7 @@
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
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -184,6 +184,12 @@
Warns about range-based loop with a loop variable of const ref type where the
type of the variable does not match the one returned by the iterator.
+- New `performance-inefficient-string-concatenation
+ `_ check
+
+ This check is to warn about the performance overhead arising from concatenating
+ strings, using the ``operator+``, instead of ``operator+=``.
+
- New `performance-unnecessary-value-param
`_ check
Index: clang-tidy/performance/Performan
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni added inline comments. Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:67 @@ +66,3 @@ +Finder->addMatcher( +exprWithCleanups(anyOf(hasDescendant(AssingOperator), + hasDescendant(PlusOperatorMatcher))), alexfh wrote: > 1. The `anyOf(hasAncestor(A), hasAncestor(B), ...)` construct is still there. > Please replace it with `hasAncestor(anyOf(A, B, ...))`. > 2. Is there really no way to change from hasDescendant / hasAncestor to more > strict patterns? I believe, running the check on LLVM doesn't help finding > performance issues, since LLVM specifically avoids this pattern by using > Twine. # I was trying to write it like you suggested since the beginning, but it says it cannot deduce the template parameter that way, what's more interesting is that clang-query accepts it. # It seems to me that the assignment operator is (always?) direct child of `exprWithCleanups`, but I'm not totally sure of that. As for the + operator it could be anywhere among the children. http://reviews.llvm.org/D20196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:41
@@ +40,3 @@
+
+ const auto PlusOperatorMatcher =
+ cxxOperatorCallExpr(
s/Matcher//
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:48
@@ +47,3 @@
+
+ const auto AssingOperator = cxxOperatorCallExpr(
+ hasOverloadedOperatorName("="), hasDescendant(BasicStringPlusOperator),
s/Assing/Assign/
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:57
@@ +56,3 @@
+
+ if (!StrictMode) {
+Finder->addMatcher(
Please avoid unnecessary negation by putting the positive branch first, this
will make the logic slightly simpler.
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:67
@@ +66,3 @@
+Finder->addMatcher(
+exprWithCleanups(anyOf(hasDescendant(AssingOperator),
+ hasDescendant(PlusOperatorMatcher))),
1. The `anyOf(hasAncestor(A), hasAncestor(B), ...)` construct is still there.
Please replace it with `hasAncestor(anyOf(A, B, ...))`.
2. Is there really no way to change from hasDescendant / hasAncestor to more
strict patterns? I believe, running the check on LLVM doesn't help finding
performance issues, since LLVM specifically avoids this pattern by using Twine.
http://reviews.llvm.org/D20196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni updated this revision to Diff 59661.
bittnerbarni marked 6 inline comments as done.
bittnerbarni added a comment.
Removed the unnecessary hasDescendant calls and simplified the checker as
suggested. Tested on LLVM codebase, with minor improvements in speed (~1%).
http://reviews.llvm.org/D20196
Files:
clang-tidy/performance/CMakeLists.txt
clang-tidy/performance/InefficientStringConcatenationCheck.cpp
clang-tidy/performance/InefficientStringConcatenationCheck.h
clang-tidy/performance/PerformanceTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
test/clang-tidy/performance-inefficient-string-concatenation.cpp
Index: test/clang-tidy/performance-inefficient-string-concatenation.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-inefficient-string-concatenation.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s performance-inefficient-string-concatenation %t
+
+namespace std {
+template
+class basic_string {
+public:
+ basic_string() {}
+ ~basic_string() {}
+ basic_string *operator+=(const basic_string &) {}
+ friend basic_string operator+(const basic_string &, const basic_string &) {}
+};
+typedef basic_string string;
+typedef basic_string 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;
+}
Index: docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
@@ -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 < 2; ++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 < 2; ++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));
+ }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -102,6 +102,7 @@
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
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -184,6 +184,12 @@
Warns about range-based loop with a loop variable of const ref type where the
type of the variable does not match the one returned by the iterator.
+- New `performance-inefficient-string-concatenation
+ `_ check
+
+ This check is to warn about the performance overhead arising from concatenating
+ strings, using the `
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:31
@@ +30,3 @@
+MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus) return;
+
clang-format -style=file, please.
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:66
@@ +65,3 @@
+
+Finder->addMatcher(WholeMatcher, this);
+Finder->addMatcher(exprWithCleanups(SurroundLoopMatcher,
Each additional matcher has a certain overhead, so it's usually beneficial to
merge matchers, if they share common parts. In this case, matchers can be
rearranged like this:
Finder->addMatcher(exprWithCleanups(
hasAncestor(anyOf(cxxForRangeStmt(), whileStmt(), forStmt())),
anyOf(hasDescendant(AssingOperator), hasDescendant(PlusOperatorMatcher;
However, the really serious problem with these matchers is the liberal usage of
`hasDescendant` matcher, which traverses the whole subtree. Nested
`hasDescendant` matchers are even worse. Note that the subtree of a statement
can be rather large, especially if you consider lambdas. Apart from performance
issues, `hasDescendant` and `hasAncestor` can yield unexpected results, in
particular, when lambdas or local classes are in play. It's always better to
use `has`, if you only need to match direct children and specifically match
intermediate nodes, if the descendant you're interested in is always on a fixed
depths.
Please try running your check (and maybe a few others for comparison, you can
use `clang-tidy -enable-check-profile` to get the run time) on a few large
translation units to estimate its performance and measure the improvement after
changing the code.
Comment at:
docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst:6
@@ +5,3 @@
+
+The problem
+---
`The problem` subheader is the only one here. It doesn't seem like it helps
making the document more structured.
Comment at:
docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst:8
@@ +7,3 @@
+---
+This check is to warn about the performance overhead arising from
concatenating strings, using the ``operator+``, for instance:
+
s/is to warn/warns/
s/strings,/strings/
Comment at:
docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst:20
@@ +19,3 @@
+ std::string a("Foo"), b("Baz");
+ for(int i = 0; i < 2; ++i)
+ {
Please make the code snippets use LLVM formatting for consistency. In
particular, add a space after `for` and remove the line break before the
opening brace.
Comment at:
docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst:39
@@ +38,3 @@
+
+ void f(const std::string&){}
+ std::string a("Foo"), b("Baz");
Add a space before the opening brace.
http://reviews.llvm.org/D20196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni updated this revision to Diff 58054.
http://reviews.llvm.org/D20196
Files:
clang-tidy/performance/CMakeLists.txt
clang-tidy/performance/InefficientStringConcatenationCheck.cpp
clang-tidy/performance/InefficientStringConcatenationCheck.h
clang-tidy/performance/PerformanceTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
test/clang-tidy/performance-inefficient-string-concatenation.cpp
Index: test/clang-tidy/performance-inefficient-string-concatenation.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-inefficient-string-concatenation.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s performance-inefficient-string-concatenation %t
+
+namespace std {
+template
+class basic_string {
+public:
+ basic_string() {}
+ ~basic_string() {}
+ basic_string *operator+=(const basic_string &) {}
+ friend basic_string operator+(const basic_string &, const basic_string &) {}
+};
+typedef basic_string string;
+typedef basic_string 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;
+}
Index: docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
@@ -0,0 +1,55 @@
+.. title:: clang-tidy - performance-inefficient-string-concatenation
+
+performance-inefficient-string-concatenation
+
+
+The problem
+---
+This check is to warn 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 < 2; ++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 < 2; ++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));
+ }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -102,6 +102,7 @@
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
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -184,6 +184,12 @@
Warns about range-based loop with a loop variable of const ref type where the
type of the variable does not match the one returned by the iterator.
+- New `performance-inefficient-string-concatenation
+ `_ check
+
+ This check is to warn about the performance overhead arising from concatenating
+ strings, using the ``operator+``, instead of ``operator+=``.
+
- New `performance-unnecessary-value-param
`_
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/performance/InefficientStringAdditionCheck.cpp:28
@@ +27,3 @@
+: ClangTidyCheck(Name, Context),
+ IsStrictMode(Options.get("isStrictMode", 0)) {}
+
s/IsStrictMode/StrictMode/
Comment at: clang-tidy/performance/InefficientStringAdditionCheck.cpp:31
@@ +30,3 @@
+void InefficientStringAdditionCheck::registerMatchers(MatchFinder *Finder) {
+ const auto BasicStringType =
+ hasType(cxxRecordDecl(hasName("::std::basic_string")));
This check is only useful for C++. Please add
```
if (!getLangOpts().CPlusPlus)
return;
```
Comment at: clang-tidy/performance/InefficientStringAdditionCheck.cpp:84
@@ +83,3 @@
+ const auto DiagMsg =
+ "inefficient string concatenation; use operator+= or "
+ "string::append() instead";
The root of inefficiency is in unnecessary allocations of temporary strings, so
maybe we should note this in the warning message, e.g. "string concatenation
results in allocation of unnecessary temporary strings; consider using
'operator+=' or 'string::append()' instead".
Comment at: clang-tidy/performance/InefficientStringAdditionCheck.h:24
@@ +23,3 @@
+///
http://clang.llvm.org/extra/clang-tidy/checks/performance-inefficient-string-addition.html
+
+class InefficientStringAdditionCheck : public ClangTidyCheck {
Please remove the empty line.
Comment at: clang-tidy/performance/InefficientStringAdditionCheck.h:25
@@ +24,3 @@
+
+class InefficientStringAdditionCheck : public ClangTidyCheck {
+ public:
s/Addition/Concatenation/?
Comment at: clang-tidy/performance/InefficientStringAdditionCheck.h:33
@@ +32,3 @@
+ private:
+ int IsStrictMode;
+};
`const bool StrictMode;`
Comment at:
docs/clang-tidy/checks/performance-inefficient-string-addition.rst:15
@@ +14,3 @@
+
+Instead of this structure you should use ``operator+=`` or std::string's
(std::basic_string) class member function ``append``. For instance:
+
Please enclose `std::string`, `std::basic_string` and other inline code
snippets in double backquotes.
http://reviews.llvm.org/D20196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni updated this revision to Diff 57894.
http://reviews.llvm.org/D20196
Files:
clang-tidy/performance/CMakeLists.txt
clang-tidy/performance/InefficientStringAdditionCheck.cpp
clang-tidy/performance/InefficientStringAdditionCheck.h
clang-tidy/performance/PerformanceTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/performance-inefficient-string-addition.rst
test/clang-tidy/performance-inefficient-string-addition.cpp
Index: test/clang-tidy/performance-inefficient-string-addition.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-inefficient-string-addition.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s performance-inefficient-string-addition %t
+
+namespace std {
+template
+class basic_string {
+public:
+ basic_string() {}
+ ~basic_string() {}
+ basic_string *operator+=(const basic_string &) {}
+ friend basic_string operator+(const basic_string &, const basic_string &) {}
+};
+typedef basic_string string;
+typedef basic_string 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: inefficient string concatenation
+mystr1 = mystr1 + mystr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ineff
+mystr1 = mystr2 + mystr2 + mystr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: ineff
+mystr1 = mystr2 + mystr1;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ineff
+mywstr1 = mywstr2 + mywstr1;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ineff
+mywstr1 = mywstr2 + mywstr2 + mywstr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: ineff
+
+mywstr1 = mywstr2 + mywstr2;
+mystr1 = mystr2 + mystr2;
+mystr1 += mystr2;
+f(mystr2 + mystr1);
+mystr1 = g(mystr1);
+ }
+ return 0;
+}
Index: docs/clang-tidy/checks/performance-inefficient-string-addition.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/performance-inefficient-string-addition.rst
@@ -0,0 +1,55 @@
+.. title:: clang-tidy - performance-inefficient-string-addition
+
+performance-inefficient-string-addition
+===
+
+The problem
+---
+This check is to warn 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 < 2; ++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 < 2; ++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));
+ }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -102,6 +102,7 @@
performance-faster-string-find
performance-for-range-copy
performance-implicit-cast-in-loop
+ performance-inefficient-string-addition
performance-unnecessary-copy-initialization
performance-unnecessary-value-param
readability-avoid-const-params-in-decls
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -184,6 +184,12 @@
Warns about range-based loop with a loop variable of const ref type where the
type of the variable does not match the one returned by the iterator.
+- New `performance-inefficient-string-addition
+ `_ check
+
+ This check is to warn about the performance overhead arising from concatenating
+ strings, using the ``operator+``, instead of ``operator+=``.
+
- New `performance-unnecessary-value-param
`_ check
Index: clang-tidy/performance/PerformanceTidyModule.cpp
===
--- clang-tidy/performance/PerformanceTidyModule.cpp
+++ clang-tidy/performance/PerformanceTidyModule.cpp
@@ -10,6 +10,7 @@
#inc
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
alexfh added a comment. Please create a diff with the full context: http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface Comment at: clang-tidy/performance/InefficientStringAdditionCheck.cpp:84 @@ +83,3 @@ + const auto DiagMsg = + "Inefficient string concatenation, use operator+= or " + "std::basic_string::append instead"; nit: Warning messages are not full sentences, so they should not start with a capital letter and should not end with a period. Please also change the comma to a semicolon `;`. Comment at: clang-tidy/performance/InefficientStringAdditionCheck.cpp:85 @@ +84,3 @@ + "Inefficient string concatenation, use operator+= or " + "std::basic_string::append instead"; + `std::basic_string::append` is correct, but is an unnecessary detail. I'd suggest changing this to `string::append` or to just `append()`. http://reviews.llvm.org/D20196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni updated this revision to Diff 57346.
http://reviews.llvm.org/D20196
Files:
clang-tidy/performance/CMakeLists.txt
clang-tidy/performance/InefficientStringAdditionCheck.cpp
clang-tidy/performance/InefficientStringAdditionCheck.h
clang-tidy/performance/PerformanceTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/performance-inefficient-string-addition.rst
test/clang-tidy/performance-inefficient-string-addition.cpp
Index: test/clang-tidy/performance-inefficient-string-addition.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-inefficient-string-addition.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s performance-inefficient-string-addition %t
+
+namespace std {
+template
+class basic_string {
+public:
+ basic_string() {}
+ ~basic_string() {}
+ basic_string *operator+=(const basic_string &) {}
+ friend basic_string operator+(const basic_string &, const basic_string &) {}
+};
+typedef basic_string string;
+typedef basic_string 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: Inefficient string concatenation
+mystr1 = mystr1 + mystr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Ineff
+mystr1 = mystr2 + mystr2 + mystr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: Ineff
+mystr1 = mystr2 + mystr1;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Ineff
+mywstr1 = mywstr2 + mywstr1;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Ineff
+mywstr1 = mywstr2 + mywstr2 + mywstr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: Ineff
+
+mywstr1 = mywstr2 + mywstr2;
+mystr1 = mystr2 + mystr2;
+mystr1 += mystr2;
+f(mystr2 + mystr1);
+mystr1 = g(mystr1);
+ }
+ return 0;
+}
Index: docs/clang-tidy/checks/performance-inefficient-string-addition.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/performance-inefficient-string-addition.rst
@@ -0,0 +1,55 @@
+.. title:: clang-tidy - performance-inefficient-string-addition
+
+performance-inefficient-string-addition
+===
+
+The problem
+---
+This check is to warn 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 < 2; ++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 < 2; ++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));
+ }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -102,6 +102,7 @@
performance-faster-string-find
performance-for-range-copy
performance-implicit-cast-in-loop
+ performance-inefficient-string-addition
performance-unnecessary-copy-initialization
performance-unnecessary-value-param
readability-avoid-const-params-in-decls
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -184,6 +184,12 @@
Warns about range-based loop with a loop variable of const ref type where the
type of the variable does not match the one returned by the iterator.
+- New `performance-inefficient-string-addition
+ `_ check
+
+ This check is to warn about the performance overhead arising from concatenating
+ strings, using the operator+, instead of operator+=.
+
- New `performance-unnecessary-value-param
`_ check
Index: clang-tidy/performance/PerformanceTidyModule.cpp
===
--- clang-tidy/performance/PerformanceTidyModule.cpp
+++ clang-tidy/performance/PerformanceTidyModule.cpp
@@ -10,6 +10,7 @@
#include "..
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
etienneb added inline comments.
Comment at: clang-tidy/performance/InefficientStringAdditionCheck.cpp:35
@@ +34,3 @@
+
cxxOperatorCallExpr(hasAnyArgument(ignoringImpCasts(declRefExpr(BasicStringType))),
+ hasOverloadedOperatorName("+"));
+
you should swap the two parameters
```
hasOverloadedOperatorName("+"),
hasAnyArgument(...)
```
matching 'hasOverloadedOperatorName' is less expensive
Comment at: clang-tidy/performance/InefficientStringAdditionCheck.cpp:41
@@ +40,3 @@
+ cxxOperatorCallExpr(hasDescendant(BasicStringPlusOperator),
+
hasAnyArgument(ignoringImpCasts(declRefExpr(BasicStringType))),
+ hasOverloadedOperatorName("+"))
this is not indented correctly.
could you run
```
% clang-format -style=file -i
```
Comment at: clang-tidy/performance/InefficientStringAdditionCheck.cpp:47
@@ +46,3 @@
+ const auto AssingOperator = cxxOperatorCallExpr(
+ hasOverloadedOperatorName("="),hasDescendant(BasicStringPlusOperator),
+ allOf(hasArgument(
space after ","
Comment at: clang-tidy/performance/InefficientStringAdditionCheck.cpp:54
@@ +53,3 @@
+1, stmt(hasDescendant(declRefExpr(hasDeclaration(decl(
+ equalsBoundNode("lhsStrT"
/*.bind("rhs-self")*/);
+
You forgot a comment here?
http://reviews.llvm.org/D20196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni updated this revision to Diff 57294.
bittnerbarni added a comment.
Sorry for uploading 2 line long diffs. I uploaded the whole diff now.
http://reviews.llvm.org/D20196
Files:
clang-tidy/performance/CMakeLists.txt
clang-tidy/performance/InefficientStringAdditionCheck.cpp
clang-tidy/performance/InefficientStringAdditionCheck.h
clang-tidy/performance/PerformanceTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/performance-inefficient-string-addition.rst
test/clang-tidy/performance-inefficient-string-addition.cpp
Index: test/clang-tidy/performance-inefficient-string-addition.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-inefficient-string-addition.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s performance-inefficient-string-addition %t
+
+namespace std {
+template
+class basic_string {
+public:
+ basic_string() {}
+ ~basic_string() {}
+ basic_string *operator+=(const basic_string &) {}
+ friend basic_string operator+(const basic_string &, const basic_string &) {}
+};
+typedef basic_string string;
+typedef basic_string 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: Inefficient string concatenation
+mystr1 = mystr1 + mystr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Ineff
+mystr1 = mystr2 + mystr2 + mystr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: Ineff
+mystr1 = mystr2 + mystr1;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Ineff
+mywstr1 = mywstr2 + mywstr1;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Ineff
+mywstr1 = mywstr2 + mywstr2 + mywstr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: Ineff
+
+mywstr1 = mywstr2 + mywstr2;
+mystr1 = mystr2 + mystr2;
+mystr1 += mystr2;
+f(mystr2 + mystr1);
+mystr1 = g(mystr1);
+ }
+ return 0;
+}
Index: docs/clang-tidy/checks/performance-inefficient-string-addition.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/performance-inefficient-string-addition.rst
@@ -0,0 +1,55 @@
+.. title:: clang-tidy - performance-inefficient-string-addition
+
+performance-inefficient-string-addition
+===
+
+The problem
+---
+This check is to warn 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 < 2; ++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 < 2; ++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));
+ }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -102,6 +102,7 @@
performance-faster-string-find
performance-for-range-copy
performance-implicit-cast-in-loop
+ performance-inefficient-string-addition
performance-unnecessary-copy-initialization
performance-unnecessary-value-param
readability-avoid-const-params-in-decls
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -184,6 +184,12 @@
Warns about range-based loop with a loop variable of const ref type where the
type of the variable does not match the one returned by the iterator.
+- New `performance-inefficient-string-addition
+ `_ check
+
+ This check is to warn about the performance overhead arising from concatenating
+ strings, using the operator+, instead of operator+=.
+
- New `performance-unnecessary-value-param
`_ check
Index: clang-tidy/performance/PerformanceTidyModule.cpp
===
--- clang-tidy/performance/Performa
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. The last diff has nothing but a 2-line change in the docs. http://reviews.llvm.org/D20196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni updated this revision to Diff 57181.
http://reviews.llvm.org/D20196
Files:
docs/clang-tidy/checks/performance-inefficient-string-addition.rst
Index: docs/clang-tidy/checks/performance-inefficient-string-addition.rst
===
--- docs/clang-tidy/checks/performance-inefficient-string-addition.rst
+++ docs/clang-tidy/checks/performance-inefficient-string-addition.rst
@@ -12,7 +12,7 @@
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:
+Instead of this structure you should use ``operator+=`` or std::string's
(std::basic_string) class member function ``append``. For instance:
.. code:: c++
Index: docs/clang-tidy/checks/performance-inefficient-string-addition.rst
===
--- docs/clang-tidy/checks/performance-inefficient-string-addition.rst
+++ docs/clang-tidy/checks/performance-inefficient-string-addition.rst
@@ -12,7 +12,7 @@
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:
+Instead of this structure you should use ``operator+=`` or std::string's (std::basic_string) class member function ``append``. For instance:
.. code:: c++
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni updated this revision to Diff 57185. http://reviews.llvm.org/D20196 Files: docs/clang-tidy/checks/performance-inefficient-string-addition.rst Index: docs/clang-tidy/checks/performance-inefficient-string-addition.rst === --- docs/clang-tidy/checks/performance-inefficient-string-addition.rst +++ docs/clang-tidy/checks/performance-inefficient-string-addition.rst @@ -5,7 +5,7 @@ The problem --- -This check is to warn about the performance overhead arising from concatenating strings, using the operator+, for instance: +This check is to warn about the performance overhead arising from concatenating strings, using the ``operator+``, for instance: .. code:: c++ Index: docs/clang-tidy/checks/performance-inefficient-string-addition.rst === --- docs/clang-tidy/checks/performance-inefficient-string-addition.rst +++ docs/clang-tidy/checks/performance-inefficient-string-addition.rst @@ -5,7 +5,7 @@ The problem --- -This check is to warn about the performance overhead arising from concatenating strings, using the operator+, for instance: +This check is to warn about the performance overhead arising from concatenating strings, using the ``operator+``, for instance: .. code:: c++ ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:191 @@ +190,3 @@ + This check is to warn about the performance overhead arising from concatenating + strings, using the operator+, instead of operator+=. + Please highlight operator+ and operator+= with ``. http://reviews.llvm.org/D20196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
bittnerbarni updated this revision to Diff 57172.
http://reviews.llvm.org/D20196
Files:
clang-tidy/performance/InefficientStringAdditionCheck.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/performance-inefficient-string-addition.rst
test/clang-tidy/performance-inefficient-string-addition.cpp
Index: test/clang-tidy/performance-inefficient-string-addition.cpp
===
--- test/clang-tidy/performance-inefficient-string-addition.cpp
+++ test/clang-tidy/performance-inefficient-string-addition.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s performance-inefficient-string-addition %t -- -- -std=c++11
+// RUN: %check_clang_tidy %s performance-inefficient-string-addition %t
namespace std {
template
@@ -24,11 +24,11 @@
f(mystr1 + mystr2 + mystr1);
// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: Inefficient string concatenation
mystr1 = mystr1 + mystr2;
-// CHECK-MESSAGES: :[[@LINE-1]]:2: warning: Ineff
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Ineff
mystr1 = mystr2 + mystr2 + mystr2;
-// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: Ineff
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: Ineff
mystr1 = mystr2 + mystr1;
-// CHECK-MESSAGES: :[[@LINE-1]]:2: warning: Ineff
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Ineff
mywstr1 = mywstr2 + mywstr1;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Ineff
mywstr1 = mywstr2 + mywstr2 + mywstr2;
@@ -41,4 +41,4 @@
mystr1 = g(mystr1);
}
return 0;
-}
\ No newline at end of file
+}
Index: docs/clang-tidy/checks/performance-inefficient-string-addition.rst
===
--- docs/clang-tidy/checks/performance-inefficient-string-addition.rst
+++ docs/clang-tidy/checks/performance-inefficient-string-addition.rst
@@ -1,24 +1,22 @@
.. title:: clang-tidy - performance-inefficient-string-addition
performance-inefficient-string-addition
-=
+===
The problem
---
+---
This check is to warn about the performance overhead arising from concatenating strings, using the operator+, for instance:
.. code:: c++
-
-std::string a = "Foo", b = "Bar";
+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:
+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";
+ std::string a("Foo"), b("Baz");
for(int i = 0; i < 2; ++i)
{
a = a + "Bar" + b;
@@ -28,7 +26,7 @@
.. code:: c++
- std::string a = "Foo", b = "Baz";
+ std::string a("Foo"), b("Baz");
for(int i = 0; i < 2; ++i)
{
a.append("Bar").append(b);
@@ -39,19 +37,19 @@
.. code:: c++
void f(const std::string&){}
- std::string a = "Foo", b = "Baz";
+ std::string a("Foo"), b("Baz");
void g()
{
- f(a+"Bar"+b);
+ f(a + "Bar" + b);
}
In a slightly more efficient way like:
.. code:: c++
void f(const std::string&){}
- std::string a = "Foo", b = "Baz";
+ std::string a("Foo"), b("Baz");
void g()
{
f(std::string(a).append("Bar").append(b));
- }
\ No newline at end of file
+ }
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -184,6 +184,12 @@
Warns about range-based loop with a loop variable of const ref type where the
type of the variable does not match the one returned by the iterator.
+- New `performance-inefficient-string-addition
+ `_ check
+
+ This check is to warn about the performance overhead arising from concatenating
+ strings, using the operator+, instead of operator+=.
+
- New `performance-unnecessary-value-param
`_ check
Index: clang-tidy/performance/InefficientStringAdditionCheck.cpp
===
--- clang-tidy/performance/InefficientStringAdditionCheck.cpp
+++ clang-tidy/performance/InefficientStringAdditionCheck.cpp
@@ -28,22 +28,22 @@
IsStrictMode(Options.get("isStrictMode", 0)) {}
void InefficientStringAdditionCheck::registerMatchers(MatchFinder *Finder) {
- auto BasicStringType = hasType(cxxRecordDecl(hasName("::std::basic_string")));
+ const auto BasicStringType = hasType(cxxRecordDecl(hasName("::std::basic_string")));
- auto BasicStringPlusOperator =
+ const auto BasicStringPlusOperator =
cxxOperatorCallExpr(hasAnyArgument(ignoringImpCasts(declRefExpr(Bas
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
etienneb added a subscriber: etienneb.
etienneb added a comment.
drive-by, some nits.
Comment at: clang-tidy/performance/InefficientStringAdditionCheck.cpp:31
@@ +30,3 @@
+void InefficientStringAdditionCheck::registerMatchers(MatchFinder *Finder) {
+ auto BasicStringType =
hasType(cxxRecordDecl(hasName("::std::basic_string")));
+
nits: I kind of prefer :
const auto BasicStringType
here and below.
Comment at: clang-tidy/performance/InefficientStringAdditionCheck.cpp:74
@@ +73,3 @@
+Finder->addMatcher(WholeMatcher, this);
+
+Finder->addMatcher(exprWithCleanups(hasDescendant(PlusOperatorMatcher),
nits: remove this line
Comment at: docs/clang-tidy/checks/performance-inefficient-string-addition.rst:4
@@ +3,3 @@
+performance-inefficient-string-addition
+=
+
line with "===" should be the same length than title.
Comment at:
docs/clang-tidy/checks/performance-inefficient-string-addition.rst:11
@@ +10,3 @@
+.. code:: c++
+
+
nits: delete one blank line
Comment at:
docs/clang-tidy/checks/performance-inefficient-string-addition.rst:19
@@ +18,3 @@
+.. code:: c++
+
+
remove extra blank line (only one)
Comment at:
docs/clang-tidy/checks/performance-inefficient-string-addition.rst:45
@@ +44,3 @@
+ {
+ f(a+"Bar"+b);
+ }
nits: spaces between operator "+"
http://reviews.llvm.org/D20196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko added a comment. Please mention this check in docs/ReleaseNotes.rst (in alphabetical order). Comment at: docs/clang-tidy/checks/performance-inefficient-string-addition.rst:8 @@ +7,3 @@ +-- +This check is to warn about the performance overhead arising from concatenating strings, using the operator+, for instance: + Please highlight operator+ and other class/methods with ``. Same for other places in documentation. Comment at: docs/clang-tidy/checks/performance-inefficient-string-addition.rst:21 @@ +20,3 @@ + + std::string a = "Foo", b = "Baz"; + for(int i = 0; i < 2; ++i) Please use constructor initialization instead of assignment. Comment at: docs/clang-tidy/checks/performance-inefficient-string-addition.rst:58 @@ +57,1 @@ + } \ No newline at end of file Please add new line. Comment at: test/clang-tidy/performance-inefficient-string-addition.cpp:1 @@ +1,2 @@ +// RUN: %check_clang_tidy %s performance-inefficient-string-addition %t -- -- -std=c++11 + Does it really need to be C++11? Comment at: test/clang-tidy/performance-inefficient-string-addition.cpp:45 @@ +44,1 @@ +} \ No newline at end of file Please add new line. http://reviews.llvm.org/D20196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
