[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