[PATCH] D16962: clang-tidy: avoid std::bind

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: clang-tidy/readability/AvoidStdBindCheck.cpp:116
@@ +115,3 @@
+  Finder->addMatcher(
+      callExpr(callee(namedDecl(isStdBind())),
+               hasArgument(0, declRefExpr(to(functionDecl().bind("f")))))
----------------
alexfh wrote:
> You should be able to use `hasName("std::bind")` instead.
I think that has to be `hasName("::std::bind")` to support `namespace mine { namespace std { void bind(); } }`

================
Comment at: clang-tidy/readability/AvoidStdBindCheck.cpp:126
@@ +125,3 @@
+
+  if (!getLangOpts().CPlusPlus14) // Need C++14 for generic lambdas.
+    return;
----------------
Might as well move this to the top of check() since there's no need to create the diagnostic or find the matched decl unless this check passes.

================
Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:37
@@ -35,1 +36,3 @@
+    CheckFactories.registerCheck<AvoidStdBindCheck>(
+        "readability-avoid-std-bind");
     CheckFactories.registerCheck<BracesAroundStatementsCheck>(
----------------
I kind of wonder if this should be in modernize instead of readability? Tough call, and I'm fine with either place, just wanted to pose the question.

================
Comment at: docs/clang-tidy/checks/readability-avoid-std-bind.rst:9
@@ +8,3 @@
+
+Right now it only handles free functions not member functions.
+
----------------
alexfh wrote:
> Add a comma before "not"?
or "and not" instead of a comma, if verbosity is your thing.

================
Comment at: test/clang-tidy/readability-avoid-std-bind.cpp:1
@@ +1,2 @@
+// RUN: %check_clang_tidy %s readability-avoid-std-bind %t -- -- -std=c++14
+
----------------
Can you clang-format this file as well?

================
Comment at: test/clang-tidy/readability-avoid-std-bind.cpp:21
@@ +20,3 @@
+
+// CHECK-FIXES: auto clj = [] { return add(2, 2); };
+
----------------
This should immediately follow the CHECK-MESSAGES in the same compound statement (here and elsewhere).


http://reviews.llvm.org/D16962