Improve modernize-avoid-bind to support more types of expressions

[zturner]

Summary

Previously modernize-avoid-bind only supported the case where the function to call was a FunctionDecl. This patch makes it support arbitrary expressions, including functors, member functions, and combinations thereof.

This change actually *simplifies* the code rather than complicates it, because it assumes that the first argument to std::bind() is always a callable, otherwise it wouldn't even compile. So rather than limiting ourselves to DeclRefExprs, we just accept any kind of expression for the first argument. Fixits are still only applied in the same set of limited cases as before, although I plan to improve this in followup patches.

[zturner]

Minor cleanup -- moved isFixitSupported logic to its own function.

[zturner]

I have a more comprehensive version of this patch that I'll upload separately.

clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp

static bool isPlaceHolderIndexRepeated(const ArrayRef<BindArgument> Args) {
   }
   return false;
 }
 
 void AvoidBindCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus14) // Need C++14 for generic lambdas.
     return;
 
-  Finder->addMatcher(
-      callExpr(
-          callee(namedDecl(hasName("::std::bind"))),
-          hasArgument(0, declRefExpr(to(functionDecl().bind("f"))).bind("ref")))
+  Finder->addMatcher(callExpr(callee(namedDecl(hasName("::std::bind"))),
+                              hasArgument(0, expr().bind("ref")))
                          .bind("bind"),
                      this);
 }
 
-void AvoidBindCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *MatchedDecl = Result.Nodes.getNodeAs<CallExpr>("bind");
-  auto Diag = diag(MatchedDecl->getBeginLoc(), "prefer a lambda to std::bind");
-
-  const auto Args = buildBindArguments(Result, MatchedDecl);
-
+static bool isFixitSupported(const Expr *Expr, ArrayRef<BindArgument> Args) {
   // Do not attempt to create fixits for nested call expressions.
   // FIXME: Create lambda capture variables to capture output of calls.
   // NOTE: Supporting nested std::bind will be more difficult due to placeholder
   // sharing between outer and inner std:bind invocations.
   if (llvm::any_of(Args,
                    [](const BindArgument &B) { return B.Kind == BK_CallExpr; }))
-    return;
+    return false;
 
   // Do not attempt to create fixits when placeholders are reused.
   // Unused placeholders are supported by requiring C++14 generic lambdas.
   // FIXME: Support this case by deducing the common type.
   if (isPlaceHolderIndexRepeated(Args))
-    return;
-
-  const auto *F = Result.Nodes.getNodeAs<FunctionDecl>("f");
+    return false;
 
-  // std::bind can support argument count mismatch between its arguments and the
-  // bound function's arguments. Do not attempt to generate a fixit for such
-  // cases.
+  if (const auto *DRE = llvm::dyn_cast<DeclRefExpr>(Expr)) {
+    const auto *FD = llvm::dyn_cast_or_null<FunctionDecl>(DRE->getDecl());
+    // std::bind can support argument count mismatch between its arguments and
+    // the bound function's arguments. Do not attempt to generate a fixit for
+    // such cases.
     // FIXME: Support this case by creating unused lambda capture variables.
-  if (F->getNumParams() != Args.size())
+    if (FD && (FD->getNumParams() == Args.size()))
+      return true;
+  }
+
+  // Do not attempt to create fixits for generalized call expressions.
+  return false;
+}
+
+void AvoidBindCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<CallExpr>("bind");
+  auto Diag = diag(MatchedDecl->getBeginLoc(), "prefer a lambda to std::bind");
+
+  const auto Args = buildBindArguments(Result, MatchedDecl);
+
+  const auto *Ref = Result.Nodes.getNodeAs<Expr>("ref");
+  if (!isFixitSupported(Ref, Args))
     return;
 
   std::string Buffer;
   llvm::raw_string_ostream Stream(Buffer);
 
   bool HasCapturedArgument = llvm::any_of(
       Args, [](const BindArgument &B) { return B.Kind == BK_Other; });
-  const auto *Ref = Result.Nodes.getNodeAs<DeclRefExpr>("ref");
   Stream << "[" << (HasCapturedArgument ? "=" : "") << "]";
   addPlaceholderArgs(Args, Stream);
   Stream << " { return ";
   Ref->printPretty(Stream, nullptr, Result.Context->getPrintingPolicy());
   Stream << "(";
   addFunctionCallArgs(Args, Stream);
   Stream << "); }";
 
   Diag << FixItHint::CreateReplacement(MatchedDecl->getSourceRange(),
                                        Stream.str());
 }
 
 } // namespace modernize
 } // namespace tidy
 } // namespace clang

clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp

 }
 
 void n() {
   auto clj = std::bind(C::add, 1, 1);
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
   // CHECK-FIXES: auto clj = [] { return C::add(1, 1); };
 }
 
+struct D {
+  void operator()(int x, int y) {}
+
+  void MemberFunction(int x) {}
+};
+
+void o() {
+  D d;
+  auto clj = std::bind(d, 1, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto clj = std::bind(d, 1, 2);
+}
+
+void p() {
+  auto clj = std::bind(D{}, 1, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto clj = std::bind(D{}, 1, 2);
+}
+
+void q() {
+  D *d;
+  auto clj = std::bind(*d, 1, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto clj = std::bind(*d, 1, 2);
+}
+
+void r() {
+  D *d;
+  auto clj = std::bind(&D::MemberFunction, d, 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto clj = std::bind(&D::MemberFunction, d, 1);
+}
+
 // Let's fake a minimal std::function-like facility.
 namespace std {
 template <typename _Tp>
 _Tp declval();
 
 template <typename _Functor, typename... _ArgTypes>
 struct __res {
   template <typename... _Args>