[PATCH] D16962: clang-tidy: avoid std::bind
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Wed May 11 10:34:15 PDT 2016
alexfh requested changes to this revision.
This revision now requires changes to proceed.
================
Comment at: clang-tidy/modernize/AvoidBindCheck.cpp:27
@@ +26,3 @@
+ StringRef Tokens;
+ BindArgumentKind Kind = BK_Other;
+ size_t PlaceHolderIndex = 0;
----------------
I wonder whether VS2013 supports inline member initializers?
================
Comment at: clang-tidy/modernize/AvoidBindCheck.cpp:77
@@ +76,3 @@
+ for (size_t I = 1; I <= PlaceholderCount; ++I) {
+ Stream << Delimiter << "auto &&"
+ << " arg" << I;
----------------
clang-format, please. Also, please join the two string literals here.
================
Comment at: clang-tidy/modernize/AvoidBindCheck.cpp:116
@@ +115,3 @@
+void AvoidBindCheck::check(const MatchFinder::MatchResult &Result) {
+ if (!getLangOpts().CPlusPlus14) // Need C++14 for generic lambdas.
+ return;
----------------
This check should be done in `registerMatchers` to avoid unnecessary work.
================
Comment at: clang-tidy/modernize/AvoidBindCheck.cpp:161
@@ +160,3 @@
+ MatchedDecl->getLocStart(),
+ Lexer::getLocForEndOfToken(MatchedDecl->getLocEnd(), 0,
+ *Result.SourceManager,
----------------
I don't think you need `getLocForEndOfToken`, since `FixItHint::CreateReplacement(SourceRange)` treats the range as a token range.
================
Comment at: docs/clang-tidy/checks/modernize-avoid-bind.rst:26
@@ +25,3 @@
+
+.. code:: C++
+ void f() {
----------------
`..code:: C++` directive should be followed by an empty line. Please check the documentation builds without warnings.
================
Comment at: docs/clang-tidy/checks/modernize-avoid-bind.rst:32
@@ +31,3 @@
+
+We created this check because ``std::bind`` can be hard to read and can result
+in larger object files and binaries due to type information that will not be
----------------
s/We created this check because //
================
Comment at: docs/clang-tidy/checks/readability-avoid-std-bind.rst:13
@@ +12,3 @@
+
+.. code:: C++
+ int add(int x, int y) { return x + y; }
----------------
alexfh wrote:
> `.. code::` should be followed by an empty line.
>
> Please also verify the documentation can be built without errors. On Ubuntu this boils down to:
>
> 1. Install sphinx:
>
> $ sudo apt-get install sphinx-common
>
> 2. Enable `LLVM_BUILD_DOCS` and maybe some other options in cmake.
>
> 3. `ninja docs-clang-tools-html` (or something similar, if you use make).
This comment is not addressed.
================
Comment at: test/clang-tidy/modernize-avoid-bind.cpp:25
@@ +24,3 @@
+ auto clj = std::bind(add, x, y);
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+ // CHECK-FIXES: auto clj = [=] { return add(x, y); };
----------------
Remove `[modernize-avoid-bind]` from all CHECK-MESSAGES lines except for the first one.
http://reviews.llvm.org/D16962
More information about the cfe-commits
mailing list