zturner created this revision.
zturner added reviewers: aaron.ballman, dblaikie, jbcoe, NoQ.
Herald added a subscriber: xazax.hun.

This represents largely a full re-write of modernize-avoid-bind, adding 
significant new functionality in the process.  In particular:

1. Both boost::bind and std::bind are now supported
2. Function objects are supported in addition to functions
3. Member functions are supported
4. Nested calls are supported using capture-init syntax
5. `std::ref()` and `boost::ref()` are now recognized, and will capture by 
reference.
6. Rather than capturing with a global `=`, we now build up an individual 
capture list that is both necessary and sufficient for the call.
7. Fixits are supported in a much larger variety of scenarios than before.

All previous tests pass under the re-write, but a large number of new tests 
have been added as well.

I don't know who the best person to review this is, so I've put a couple of 
people on here.  Feel free to re-direct if there's someone better.


https://reviews.llvm.org/D70368

Files:
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
@@ -8,75 +8,51 @@
 template <class Fp, class... Arguments>
 bind_rt<Fp, Arguments...> bind(Fp &&, Arguments &&...);
 }
+
+template <typename T>
+T ref(T &t);
 }
 
-int add(int x, int y) { return x + y; }
+namespace boost {
+template <class Fp, class... Arguments>
+class bind_rt {};
 
-void f() {
-  auto clj = std::bind(add, 2, 2);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
-  // CHECK-FIXES: auto clj = [] { return add(2, 2); };
-}
+template <class Fp, class... Arguments>
+bind_rt<Fp, Arguments...> bind(const Fp &, Arguments...);
+} // namespace boost
 
-void g() {
-  int x = 2;
-  int y = 2;
-  auto clj = std::bind(add, x, y);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto clj = [=] { return add(x, y); };
-}
+namespace C {
+int add(int x, int y) { return x + y; }
+} // namespace C
 
-struct placeholder {};
-placeholder _1;
-placeholder _2;
+struct Foo {
+  static int add(int x, int y) { return x + y; }
+};
 
-void h() {
-  int x = 2;
-  auto clj = std::bind(add, x, _1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto clj = [=](auto && arg1) { return add(x, arg1); };
-}
+struct D {
+  D() = default;
+  void operator()(int x, int y) const {}
 
-struct A;
-struct B;
-bool ABTest(const A &, const B &);
+  void MemberFunction(int x) {}
 
-void i() {
-  auto BATest = std::bind(ABTest, _2, _1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto BATest = [](auto && arg1, auto && arg2) { return ABTest(arg2, arg1); };
-}
+  static D *create();
+};
 
-void j() {
-  auto clj = std::bind(add, 2, 2, 2);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // No fix is applied for argument mismatches.
-  // CHECK-FIXES: auto clj = std::bind(add, 2, 2, 2);
-}
+struct F {
+  F(int x) {}
+  ~F() {}
 
-void k() {
-  auto clj = std::bind(add, _1, _1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // No fix is applied for reused placeholders.
-  // CHECK-FIXES: auto clj = std::bind(add, _1, _1);
-}
+  int get() { return 42; }
+};
 
-void m() {
-  auto clj = std::bind(add, 1, add(2, 5));
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // No fix is applied for nested calls.
-  // CHECK-FIXES: auto clj = std::bind(add, 1, add(2, 5));
-}
+void UseF(F);
 
-namespace C {
-  int add(int x, int y){ return x + y; }
-}
+struct placeholder {};
+placeholder _1;
+placeholder _2;
 
-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); };
-}
+int add(int x, int y) { return x + y; }
+int addThree(int x, int y, int z) { return x + y + z; }
 
 // Let's fake a minimal std::function-like facility.
 namespace std {
@@ -114,10 +90,188 @@
   void Reset(std::function<void()>);
 };
 
-void test(Thing *t) {
+int GlobalVariable = 42;
+
+struct TestCaptureByValueStruct {
+  int MemberVariable;
+  static int StaticMemberVariable;
+  F MemberStruct;
+
+  void testCaptureByValue(int Param, F f) {
+    int x = 3;
+    int y = 4;
+    auto AAA = std::bind(add, x, y);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+    // CHECK-FIXES: auto AAA = [x, y] { return add(x, y); };
+
+    // When the captured variable is repeated, it should only appear in the capture list once.
+    auto BBB = std::bind(add, x, x);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+    // CHECK-FIXES: auto BBB = [x] { return add(x, x); };
+
+    int LocalVariable;
+    // Global variables shouldn't be captured at all, and members should be captured through this.
+    auto CCC = std::bind(add, MemberVariable, GlobalVariable);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+    // CHECK-FIXES: auto CCC = [this] { return add(MemberVariable, GlobalVariable); };
+
+    // Static member variables shouldn't be captured, but locals should
+    auto DDD = std::bind(add, TestCaptureByValueStruct::StaticMemberVariable, LocalVariable);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+    // CHECK-FIXES: auto DDD = [LocalVariable] { return add(TestCaptureByValueStruct::StaticMemberVariable, LocalVariable); };
+
+    auto EEE = std::bind(add, Param, Param);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+    // CHECK-FIXES: auto EEE = [Param] { return add(Param, Param); };
+
+    // The signature of boost::bind() is different, and causes
+    // CXXBindTemporaryExprs to be created in certain cases.  So let's test
+    // those here.
+    auto FFF = boost::bind(UseF, f);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to boost::bind [modernize-avoid-bind]
+    // CHECK-FIXES: auto FFF = [f] { return UseF(f); };
+
+    auto GGG = boost::bind(UseF, MemberStruct);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to boost::bind [modernize-avoid-bind]
+    // CHECK-FIXES: auto GGG = [this] { return UseF(MemberStruct); };
+  }
+};
+
+void testLiteralParameters() {
+  auto AAA = std::bind(add, 2, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+  // CHECK-FIXES: auto AAA = [] { return add(2, 2); };
+
+  auto BBB = std::bind(addThree, 2, 3, 4);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+  // CHECK-FIXES: auto BBB = [] { return addThree(2, 3, 4); };
+}
+
+void testCaptureByReference() {
+  int x = 2;
+  int y = 2;
+  auto AAA = std::bind(add, std::ref(x), std::ref(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto AAA = [&x, &y] { return add(x, y); };
+
+  auto BBB = std::bind(add, std::ref(x), y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto BBB = [&x, y] { return add(x, y); };
+
+  auto CCC = std::bind(add, y, std::ref(x));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto CCC = [y, &x] { return add(y, x); };
+}
+
+void testCaptureByInitExpression() {
+  int x = 42;
+  auto AAA = std::bind(add, x, F(x).get());
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto AAA = [x, capture1 = F(x).get()] { return add(x, capture1); };
+}
+
+void testPlaceholders() {
+  int x = 2;
+  auto AAA = std::bind(add, x, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto AAA = [x](auto && PH1) { return add(x, PH1); };
+
+  auto BBB = std::bind(add, _2, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto BBB = [](auto && PH1, auto && PH2) { return add(PH2, PH1); };
+
+  // No fix is applied for reused placeholders.
+  auto CCC = std::bind(add, _1, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto CCC = std::bind(add, _1, _1);
+}
+
+void testGlobalFunctions() {
+  auto AAA = std::bind(C::add, 1, 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto AAA = [] { return C::add(1, 1); };
+
+  auto BBB = std::bind(Foo::add, 1, 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto BBB = [] { return Foo::add(1, 1); };
+
+  // The & should get removed inside of the lambda body.
+  auto CCC = std::bind(&C::add, 1, 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto CCC = [] { return C::add(1, 1); };
+
+  auto DDD = std::bind(&Foo::add, 1, 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto DDD = [] { return Foo::add(1, 1); };
+
+  auto EEE = std::bind(&add, 1, 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto EEE = [] { return add(1, 1); };
+}
+
+void testCapturedSubexpressions() {
+  int x = 3;
+  int y = 3;
+
+  auto AAA = std::bind(add, 1, add(2, 5));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // Results of nested calls are captured by value.
+  // CHECK-FIXES: auto AAA = [capture1 = add(2, 5)] { return add(1, capture1); };
+
+  auto BBB = std::bind(add, x, add(y, 5));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // Results of nested calls are captured by value.
+  // CHECK-FIXES: auto BBB = [x, capture1 = add(y, 5)] { return add(x, capture1); };
+}
+
+void testFunctionObjects() {
+  D d;
+  D *e = nullptr;
+  auto AAA = std::bind(d, 1, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto AAA = [d] { return d(1, 2); }
+
+  auto BBB = std::bind(*e, 1, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto BBB = [e] { return (*e)(1, 2); }
+
+  auto CCC = std::bind(D{}, 1, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto CCC = [] { return D{}(1, 2); }
+
+  auto DDD = std::bind(D(), 1, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto DDD = [] { return D()(1, 2); }
+
+  auto EEE = std::bind(*D::create(), 1, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto EEE = [Func = *D::create()] { return Func(1, 2); };
+}
+
+struct E {
+  void MemberFunction(int x) {}
+
+  void testMemberFunctions() {
+    auto CCC = std::bind(&E::MemberFunction, this, 1);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+    // CHECK-FIXES: auto CCC = [this] { MemberFunction(1); };
+
+    D *d;
+    D dd;
+    auto AAA = std::bind(&D::MemberFunction, d, 1);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+    // CHECK-FIXES: auto AAA = [d] { d->MemberFunction(1); };
+
+    auto BBB = std::bind(&D::MemberFunction, &dd, 1);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+    // CHECK-FIXES: auto BBB = [ObjectPtr = &dd] { ObjectPtr->MemberFunction(1); };
+  }
+};
+
+void testStdFunction(Thing *t) {
   Callback cb;
   if (t)
     cb.Reset(std::bind(UseThing, t));
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: cb.Reset([=] { return UseThing(t); });
+  // CHECK-FIXES: cb.Reset([t] { return UseThing(t); });
 }
Index: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.h
+++ clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.h
@@ -23,8 +23,7 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/modernize-avoid-std-bind.html
 class AvoidBindCheck : public ClangTidyCheck {
 public:
-  AvoidBindCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  AvoidBindCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 };
Index: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
@@ -14,11 +14,12 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
@@ -33,46 +34,331 @@
 
 namespace {
 
-enum BindArgumentKind { BK_Temporary, BK_Placeholder, BK_CallExpr, BK_Other };
+enum BindArgumentKind {
+  BK_Temporary,
+  BK_Placeholder,
+  BK_CallExpr,
+  BK_ObjectPtr,
+  BK_Other
+};
+enum CaptureMode { CM_None, CM_ByRef, CM_ByValue, CM_InitExpression };
+
+enum CallableType {
+  CT_Other,                  // unknown
+  CT_Function,               // global or static function
+  CT_MemberFunction,         // member function with implicit this
+  CT_Object,                 // object with operator()
+  CT_StandardFunctionObject, // std::function<> or boost::function<>
+};
+
+enum CallableMaterializationKind {
+  CMK_Other,       // unknown
+  CMK_Function,    // callable is the name of a member or non-member function.
+  CMK_VariableRef, // callable is a simple expression involving a global or
+                   // local variable.
+  CMK_CallExpression, // callable is obtained as the result of a call expression
+};
 
 struct BindArgument {
-  StringRef Tokens;
+  // A rough classification of the type of expression this argument was.
   BindArgumentKind Kind = BK_Other;
+
+  // If this argument required a capture, a value indicating how it was
+  // captured.
+  CaptureMode CaptureMode = CM_None;
+
+  // The exact spelling of this argument in the source code.
+  StringRef SourceTokens;
+
+  // The identifier of the variable within the capture list.  This may be
+  // different from UsageIdentifier for example in the expression *d, where the
+  // variable is captured as d, but referred to as *d.
+  std::string CaptureIdentifier;
+
+  // If this is a placeholder or capture init expression, contains the tokens
+  // used to refer to this parameter from within the body of the lambda.
+  std::string UsageIdentifier;
+
+  // If Kind == BK_Placeholder, the index of the placeholder.
   size_t PlaceHolderIndex = 0;
+
+  // True if the argument is used inside the lambda, false otherwise.
+  bool IsUsed = false;
+
+  // The actual Expr object representing this expression.
+  const Expr *E = nullptr;
+};
+
+struct CallableInfo {
+  CallableType Type = CT_Other;
+  CallableMaterializationKind Materialization = CMK_Other;
+  CaptureMode CaptureMode = CM_None;
+  StringRef SourceTokens;
+  std::string CaptureIdentifier;
+  std::string UsageIdentifier;
+  StringRef CaptureInitializer;
+  const FunctionDecl *Decl = nullptr;
+};
+
+struct LambdaProperties {
+  CallableInfo Callable;
+
+  SmallVector<BindArgument, 4> BindArguments;
+
+  StringRef BindNamespace;
+
+  bool IsFixitSupported = false;
 };
 
 } // end namespace
 
+static const Expr *ignoreTemporariesAndImplicitCasts(const Expr *E) {
+  if (const auto *T = dyn_cast<MaterializeTemporaryExpr>(E))
+    return ignoreTemporariesAndImplicitCasts(T->GetTemporaryExpr());
+  if (const auto *T = dyn_cast<ImplicitCastExpr>(E))
+    return ignoreTemporariesAndImplicitCasts(T->getSubExpr());
+  return E;
+}
+
+static const Expr *ignoreTemporariesAndPointers(const Expr *E) {
+
+  if (const auto *T = dyn_cast<UnaryOperator>(E))
+    return ignoreTemporariesAndPointers(T->getSubExpr());
+  if (const auto *T = dyn_cast<ImplicitCastExpr>(E))
+    return ignoreTemporariesAndPointers(T->getSubExpr());
+
+  if (const auto *T = dyn_cast<MaterializeTemporaryExpr>(E))
+    return ignoreTemporariesAndPointers(T->GetTemporaryExpr());
+  return E;
+}
+
+static StringRef getSourceTextForExpr(const MatchFinder::MatchResult &Result,
+                                      const Expr *E) {
+  return Lexer::getSourceText(
+      CharSourceRange::getTokenRange(E->getBeginLoc(), E->getEndLoc()),
+      *Result.SourceManager, Result.Context->getLangOpts());
+}
+
+static std::string nextCaptureIdentifier(unsigned &CaptureIndex) {
+  std::string S;
+  llvm::raw_string_ostream OS(S);
+  OS << "capture" << CaptureIndex++;
+  OS.flush();
+  return S;
+}
+
+static bool isDeclNamed(const Decl *D, llvm::StringRef Name) {
+  ast_matchers::internal::HasNameMatcher HNM{{Name}};
+
+  return HNM.matchesNode(*dyn_cast<NamedDecl>(D));
+}
+
+static bool isCallExprNamed(const Expr *E, llvm::StringRef Name) {
+  if (const auto *M = dyn_cast<MaterializeTemporaryExpr>(E))
+    return isCallExprNamed(M->GetTemporaryExpr(), Name);
+
+  if (const auto *CE = dyn_cast<CallExpr>(E))
+    return isDeclNamed(CE->getCalleeDecl(), Name);
+
+  return false;
+}
+
+static void
+initializeBindArgumentForCallExpr(const MatchFinder::MatchResult &Result,
+                                  BindArgument &B, const CallExpr *CE,
+                                  unsigned &CaptureIndex) {
+  B.UsageIdentifier = nextCaptureIdentifier(CaptureIndex);
+
+  // std::ref(x) means to capture x by reference.
+  if (isCallExprNamed(CE, "::boost::ref") ||
+      isCallExprNamed(CE, "::std::ref")) {
+    B.Kind = BK_Other;
+    B.CaptureMode = CM_ByRef;
+    B.UsageIdentifier = getSourceTextForExpr(Result, CE->getArg(0));
+  } else {
+    B.Kind = BK_CallExpr;
+    B.CaptureMode = CM_InitExpression;
+    B.UsageIdentifier = nextCaptureIdentifier(CaptureIndex);
+  }
+  B.CaptureIdentifier = B.UsageIdentifier;
+}
+
+static bool anyDescendantIsLocal(const Stmt *Statement) {
+  if (const auto *DeclRef = llvm::dyn_cast<DeclRefExpr>(Statement)) {
+    const ValueDecl *Decl = DeclRef->getDecl();
+    if (const auto *Var = llvm::dyn_cast_or_null<VarDecl>(Decl)) {
+      if (Var->isLocalVarDeclOrParm())
+        return true;
+    }
+  } else if (const auto *ThisExpr = llvm::dyn_cast<CXXThisExpr>(Statement))
+    return true;
+
+  for (const auto *Child : Statement->children()) {
+    if (anyDescendantIsLocal(Child))
+      return true;
+  }
+
+  return false;
+}
+
+static void
+initializeBindArgumentForTemporaryExpr(BindArgument &B,
+                                       const MaterializeTemporaryExpr *MTE,
+                                       unsigned &CaptureIndex) {
+  B.Kind = BK_Temporary;
+
+  // If this is a temporary expression that is materializing the value of an
+  // integer literal or global expression, it doesn't need an init-capture and
+  // can be pasted directly into the body of the lambda.
+  if (!anyDescendantIsLocal(MTE))
+    return;
+
+  B.CaptureMode = CM_InitExpression;
+  B.UsageIdentifier = nextCaptureIdentifier(CaptureIndex);
+  B.CaptureIdentifier = B.UsageIdentifier;
+}
+
+static bool tryCaptureAsLocalVariable(const MatchFinder::MatchResult &Result,
+                                      BindArgument &B, const Expr *E) {
+  if (const auto *BTE = llvm::dyn_cast<CXXBindTemporaryExpr>(E)) {
+    if (const auto *CE = llvm::dyn_cast<CXXConstructExpr>(BTE->getSubExpr()))
+      return tryCaptureAsLocalVariable(Result, B, CE->getArg(0));
+    return false;
+  }
+
+  const auto *DRE = dyn_cast<DeclRefExpr>(ignoreTemporariesAndImplicitCasts(E));
+  if (!DRE)
+    return false;
+
+  const auto *VD = dyn_cast<VarDecl>(DRE->getDecl());
+  if (!VD || !VD->isLocalVarDeclOrParm())
+    return false;
+
+  B.CaptureMode = CM_ByValue;
+  B.UsageIdentifier = getSourceTextForExpr(Result, E);
+  B.CaptureIdentifier = B.UsageIdentifier;
+  return true;
+}
+
+static bool tryCaptureAsMemberVariable(const MatchFinder::MatchResult &Result,
+                                       BindArgument &B, const Expr *E) {
+  if (const auto *BTE = llvm::dyn_cast<CXXBindTemporaryExpr>(E)) {
+    if (const auto *CE = llvm::dyn_cast<CXXConstructExpr>(BTE->getSubExpr()))
+      return tryCaptureAsMemberVariable(Result, B, CE->getArg(0));
+    return false;
+  }
+
+  E = ignoreTemporariesAndImplicitCasts(E);
+  if (isa<CXXThisExpr>(E)) {
+    B.CaptureMode = CM_ByValue;
+    B.UsageIdentifier = getSourceTextForExpr(Result, E);
+    B.CaptureIdentifier = "this";
+    return true;
+  }
+
+  const auto *ME = dyn_cast<MemberExpr>(E);
+  if (!ME)
+    return false;
+
+  if (!ME->isLValue() || !llvm::isa<FieldDecl>(ME->getMemberDecl()))
+    return false;
+
+  B.CaptureMode = CM_ByValue;
+  B.UsageIdentifier = getSourceTextForExpr(Result, E);
+  B.CaptureIdentifier = "this";
+  return true;
+}
+
 static SmallVector<BindArgument, 4>
-buildBindArguments(const MatchFinder::MatchResult &Result, const CallExpr *C) {
+buildBindArguments(const MatchFinder::MatchResult &Result,
+                   const CallableInfo &Callable) {
   SmallVector<BindArgument, 4> BindArguments;
   llvm::Regex MatchPlaceholder("^_([0-9]+)$");
 
+  const auto *BindCall = Result.Nodes.getNodeAs<CallExpr>("bind");
+
   // Start at index 1 as first argument to bind is the function name.
-  for (size_t I = 1, ArgCount = C->getNumArgs(); I < ArgCount; ++I) {
-    const Expr *E = C->getArg(I);
-    BindArgument B;
-    if (const auto *M = dyn_cast<MaterializeTemporaryExpr>(E)) {
-      const auto *TE = M->GetTemporaryExpr();
-      B.Kind = isa<CallExpr>(TE) ? BK_CallExpr : BK_Temporary;
-    }
+  unsigned CaptureIndex = 0;
+  for (size_t I = 1, ArgCount = BindCall->getNumArgs(); I < ArgCount; ++I) {
+
+    bool IsObjectPtrArg = (I == 1 && Callable.Type == CT_MemberFunction);
+
+    const Expr *E = BindCall->getArg(I);
+    BindArgument &B = BindArguments.emplace_back();
+
+    size_t ArgIndex = I - 1;
+    if (Callable.Type == CT_MemberFunction)
+      --ArgIndex;
+
+    if (!Callable.Decl || ArgIndex < Callable.Decl->getNumParams() ||
+        IsObjectPtrArg)
+      B.IsUsed = true;
 
-    B.Tokens = Lexer::getSourceText(
-        CharSourceRange::getTokenRange(E->getBeginLoc(), E->getEndLoc()),
-        *Result.SourceManager, Result.Context->getLangOpts());
+    B.E = E;
+    B.SourceTokens = getSourceTextForExpr(Result, E);
+
+    if (IsObjectPtrArg) {
+      B.Kind = BK_ObjectPtr;
+
+      if (tryCaptureAsLocalVariable(Result, B, B.E) ||
+          tryCaptureAsMemberVariable(Result, B, B.E))
+        continue;
+
+      B.CaptureMode = CM_InitExpression;
+      B.UsageIdentifier = "ObjectPtr";
+      B.CaptureIdentifier = B.UsageIdentifier;
+      continue;
+    }
 
     SmallVector<StringRef, 2> Matches;
-    if (B.Kind == BK_Other && MatchPlaceholder.match(B.Tokens, &Matches)) {
+    if (MatchPlaceholder.match(B.SourceTokens, &Matches)) {
       B.Kind = BK_Placeholder;
       B.PlaceHolderIndex = std::stoi(Matches[1]);
+      llvm::raw_string_ostream OS(B.UsageIdentifier);
+      OS << "PH" << B.PlaceHolderIndex;
+      OS.flush();
+      B.CaptureIdentifier = B.UsageIdentifier;
+      continue;
+    }
+
+    if (const auto *CE =
+            dyn_cast<CallExpr>(ignoreTemporariesAndImplicitCasts(E))) {
+      initializeBindArgumentForCallExpr(Result, B, CE, CaptureIndex);
+      continue;
+    }
+
+    if (tryCaptureAsLocalVariable(Result, B, B.E) ||
+        tryCaptureAsMemberVariable(Result, B, B.E))
+      continue;
+
+    // If it's not something we recognize, capture it by init expression to be
+    // safe.
+    B.Kind = BK_Other;
+    if (anyDescendantIsLocal(B.E)) {
+      B.CaptureMode = CM_InitExpression;
+      B.CaptureIdentifier = nextCaptureIdentifier(CaptureIndex);
+      B.UsageIdentifier = B.CaptureIdentifier;
     }
-    BindArguments.push_back(B);
   }
   return BindArguments;
 }
 
-static void addPlaceholderArgs(const ArrayRef<BindArgument> Args,
+static size_t findPositionOfPlaceholderUse(ArrayRef<BindArgument> Args,
+                                           size_t PlaceholderIndex) {
+  for (size_t I = 0; I < Args.size(); ++I)
+    if (Args[I].PlaceHolderIndex == PlaceholderIndex)
+      return I;
+
+  return 0;
+}
+
+static void addPlaceholderArgs(const LambdaProperties &LP,
                                llvm::raw_ostream &Stream) {
+
+  ArrayRef<BindArgument> Args = LP.BindArguments;
+  if (LP.Callable.Type == CT_MemberFunction)
+    Args = Args.drop_front();
+
   auto MaxPlaceholderIt =
       std::max_element(Args.begin(), Args.end(),
                        [](const BindArgument &B1, const BindArgument &B2) {
@@ -87,20 +373,31 @@
   Stream << "(";
   StringRef Delimiter = "";
   for (size_t I = 1; I <= PlaceholderCount; ++I) {
-    Stream << Delimiter << "auto && arg" << I;
+    Stream << Delimiter << "auto &&";
+
+    size_t ArgIndex = findPositionOfPlaceholderUse(Args, I);
+
+    if (Args[ArgIndex].IsUsed)
+      Stream << " " << Args[ArgIndex].CaptureIdentifier;
     Delimiter = ", ";
   }
   Stream << ")";
 }
 
-static void addFunctionCallArgs(const ArrayRef<BindArgument> Args,
+static void addFunctionCallArgs(ArrayRef<BindArgument> Args,
                                 llvm::raw_ostream &Stream) {
   StringRef Delimiter = "";
-  for (const auto &B : Args) {
-    if (B.PlaceHolderIndex)
-      Stream << Delimiter << "arg" << B.PlaceHolderIndex;
-    else
-      Stream << Delimiter << B.Tokens;
+
+  for (int I = 0, Size = Args.size(); I < Size; ++I) {
+    const BindArgument &B = Args[I];
+
+    Stream << Delimiter;
+
+    if (B.Kind == BK_Placeholder || B.CaptureMode != CM_None)
+      Stream << B.UsageIdentifier;
+    else if (B.CaptureMode == CM_None)
+      Stream << B.SourceTokens;
+
     Delimiter = ", ";
   }
 }
@@ -116,59 +413,310 @@
   return false;
 }
 
+static std::vector<const CXXMethodDecl *>
+findCandidateCallOperators(const CXXRecordDecl *RecordDecl, size_t NumArgs) {
+  std::vector<const CXXMethodDecl *> Candidates;
+
+  for (const clang::CXXMethodDecl *Method : RecordDecl->methods()) {
+    OverloadedOperatorKind OOK = Method->getOverloadedOperator();
+
+    if (OOK != OverloadedOperatorKind::OO_Call)
+      continue;
+
+    if (Method->getNumParams() > NumArgs)
+      continue;
+
+    Candidates.push_back(Method);
+  }
+  if (!Candidates.empty())
+    return Candidates;
+
+  return Candidates;
+}
+
+static bool isFixitSupported(const CallableInfo &Callee,
+                             ArrayRef<BindArgument> Args) {
+  // Do not attempt to create fixits for nested std::bind or std::ref.
+  // Supporting nested std::bind will be more difficult due to placeholder
+  // sharing between outer and inner std::bind invocations, and std::ref
+  // requires us to capture some parameters by reference instead of by value.
+  if (llvm::any_of(Args, [](const BindArgument &B) {
+        return isCallExprNamed(B.E, "::boost::bind") ||
+               isCallExprNamed(B.E, "::std::bind");
+      })) {
+    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 false;
+
+  if (Callee.Type == CT_Other || Callee.Materialization == CMK_Other)
+    return false;
+
+  return true;
+}
+
+const FunctionDecl *getCallOperator(const CXXRecordDecl *Callable,
+                                    size_t NumArgs) {
+  std::vector<const CXXMethodDecl *> Candidates =
+      findCandidateCallOperators(Callable, NumArgs);
+  if (Candidates.size() != 1)
+    return nullptr;
+
+  return Candidates.front();
+}
+
+const FunctionDecl *
+getCallMethodDecl(const MatchFinder::MatchResult &Result, CallableType Type,
+                  CallableMaterializationKind Materialization) {
+
+  const Expr *CallExpression =
+      ignoreTemporariesAndPointers(Result.Nodes.getNodeAs<Expr>("ref"));
+
+  if (Materialization == CMK_Function) {
+    if (const auto *DRE = dyn_cast<DeclRefExpr>(CallExpression)) {
+      if (const auto *FD = llvm::dyn_cast<FunctionDecl>(DRE->getDecl()))
+        return FD;
+    }
+    return nullptr;
+  }
+
+  if (Type == CT_Object) {
+    const auto *BindCall = Result.Nodes.getNodeAs<CallExpr>("bind");
+    size_t NumArgs = BindCall->getNumArgs() - 1;
+    return getCallOperator(BindCall->getType()->getAsCXXRecordDecl(), NumArgs);
+  }
+
+  // Maybe this is an indirect call through a function pointer or something
+  // where we can't determine the exact decl.
+  return nullptr;
+}
+
+static CallableType getCallableType(const MatchFinder::MatchResult &Result) {
+  const auto *CallableExpr = Result.Nodes.getNodeAs<Expr>("ref");
+
+  QualType QT = CallableExpr->getType();
+  if (QT->isMemberFunctionPointerType())
+    return CT_MemberFunction;
+
+  if (QT->isFunctionPointerType() || QT->isFunctionReferenceType() ||
+      QT->isFunctionType())
+    return CT_Function;
+
+  if (QT->isRecordType()) {
+    const CXXRecordDecl *Decl = QT->getAsCXXRecordDecl();
+    if (!Decl)
+      return CT_Other;
+
+    if (isDeclNamed(Decl, "::boost::function") ||
+        isDeclNamed(Decl, "::std::function"))
+      return CT_StandardFunctionObject;
+
+    return CT_Object;
+  }
+
+  return CT_Other;
+}
+
+static CallableMaterializationKind
+getCallableMaterialization(const MatchFinder::MatchResult &Result) {
+  const auto *CallableExpr = Result.Nodes.getNodeAs<Expr>("ref");
+
+  const auto *NoTemporaries = ignoreTemporariesAndPointers(CallableExpr);
+
+  if (isa<CallExpr>(NoTemporaries))
+    return CMK_CallExpression;
+
+  if (isa<CXXFunctionalCastExpr>(NoTemporaries) ||
+      isa<CXXConstructExpr>(NoTemporaries))
+    return CMK_Function;
+
+  if (const auto *DRE = dyn_cast<DeclRefExpr>(NoTemporaries)) {
+    if (const auto *FD = dyn_cast<FunctionDecl>(DRE->getDecl()))
+      return CMK_Function;
+    if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl()))
+      return CMK_VariableRef;
+  }
+
+  return CMK_Other;
+}
+
+static LambdaProperties
+getLambdaProperties(const MatchFinder::MatchResult &Result) {
+  const auto *CalleeExpr = Result.Nodes.getNodeAs<Expr>("ref");
+
+  LambdaProperties LP;
+
+  const auto *Bind = Result.Nodes.getNodeAs<CallExpr>("bind");
+  const auto *Decl = llvm::dyn_cast<FunctionDecl>(Bind->getCalleeDecl());
+  const auto *NS =
+      llvm::dyn_cast<NamespaceDecl>(Decl->getEnclosingNamespaceContext());
+  while (NS->isInlineNamespace())
+    NS = llvm::dyn_cast<NamespaceDecl>(NS->getDeclContext());
+  LP.BindNamespace = NS->getName();
+
+  LP.Callable.Type = getCallableType(Result);
+  LP.Callable.Materialization = getCallableMaterialization(Result);
+  LP.Callable.Decl =
+      getCallMethodDecl(Result, LP.Callable.Type, LP.Callable.Materialization);
+  LP.Callable.SourceTokens = getSourceTextForExpr(Result, CalleeExpr);
+  if (LP.Callable.Materialization == CMK_VariableRef) {
+    LP.Callable.CaptureMode = CM_ByValue;
+    LP.Callable.UsageIdentifier = getSourceTextForExpr(Result, CalleeExpr);
+    LP.Callable.CaptureIdentifier =
+        getSourceTextForExpr(Result, ignoreTemporariesAndPointers(CalleeExpr));
+  } else if (LP.Callable.Materialization == CMK_CallExpression) {
+    LP.Callable.CaptureMode = CM_InitExpression;
+    LP.Callable.UsageIdentifier = "Func";
+    LP.Callable.CaptureIdentifier = "Func";
+    LP.Callable.CaptureInitializer = getSourceTextForExpr(Result, CalleeExpr);
+  }
+
+  LP.BindArguments = buildBindArguments(Result, LP.Callable);
+
+  LP.IsFixitSupported = isFixitSupported(LP.Callable, LP.BindArguments);
+
+  return LP;
+}
+
+static bool emitCapture(llvm::StringSet<> &CaptureSet, StringRef Delimiter,
+                        CaptureMode CM, StringRef Identifier,
+                        StringRef InitExpression, raw_ostream &Stream) {
+  if (CM == CM_None)
+    return false;
+
+  // This capture has already been emitted.
+  if (CaptureSet.count(Identifier) != 0)
+    return false;
+
+  Stream << Delimiter;
+
+  if (CM == CM_ByRef)
+    Stream << "&";
+  Stream << Identifier;
+  if (CM == CM_InitExpression)
+    Stream << " = " << InitExpression;
+
+  CaptureSet.insert(Identifier);
+  return true;
+}
+
+static void emitCaptureList(const LambdaProperties &LP,
+                            const MatchFinder::MatchResult &Result,
+                            raw_ostream &Stream) {
+  llvm::StringSet<> CaptureSet;
+  bool AnyCapturesEmitted = false;
+
+  AnyCapturesEmitted = emitCapture(CaptureSet, "", LP.Callable.CaptureMode,
+                                   LP.Callable.CaptureIdentifier,
+                                   LP.Callable.CaptureInitializer, Stream);
+
+  for (const BindArgument &B : LP.BindArguments) {
+    if (B.CaptureMode == CM_None || !B.IsUsed)
+      continue;
+
+    StringRef Delimiter = AnyCapturesEmitted ? ", " : "";
+
+    if (emitCapture(CaptureSet, Delimiter, B.CaptureMode, B.CaptureIdentifier,
+                    B.SourceTokens, Stream))
+      AnyCapturesEmitted = true;
+  }
+}
+
+static ArrayRef<BindArgument>
+getForwardedArgumentList(const LambdaProperties &P) {
+  ArrayRef<BindArgument> Args = llvm::makeArrayRef(P.BindArguments);
+  if (P.Callable.Type != CT_MemberFunction)
+    return Args;
+
+  return Args.drop_front();
+}
+AvoidBindCheck::AvoidBindCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context) {}
+
 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")))
+          callee(namedDecl(
+              anyOf(hasName("::boost::bind"), hasName("::std::bind")))),
+          hasArgument(
+              0, anyOf(expr(hasType(memberPointerType())).bind("ref"),
+                       expr(hasParent(materializeTemporaryExpr().bind("ref"))),
+                       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);
-
-  // 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;
 
-  // 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))
+  LambdaProperties LP = getLambdaProperties(Result);
+  auto Diag = diag(
+      MatchedDecl->getBeginLoc(),
+      llvm::formatv("prefer a lambda to {0}::bind", LP.BindNamespace).str());
+  if (!LP.IsFixitSupported)
     return;
 
-  const auto *F = Result.Nodes.getNodeAs<FunctionDecl>("f");
-
-  // 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())
-    return;
+  const auto *Ref = Result.Nodes.getNodeAs<Expr>("ref");
 
   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 << "[";
+  emitCaptureList(LP, Result, Stream);
+  Stream << "]";
+
+  ArrayRef<BindArgument> FunctionCallArgs =
+      llvm::makeArrayRef(LP.BindArguments);
+
+  addPlaceholderArgs(LP, Stream);
+
+  if (LP.Callable.Type == CT_Function) {
+    StringRef SourceTokens = LP.Callable.SourceTokens;
+    SourceTokens.consume_front("&");
+    Stream << " { return " << SourceTokens;
+  } else if (LP.Callable.Type == CT_MemberFunction) {
+    const auto *MethodDecl = llvm::dyn_cast<CXXMethodDecl>(LP.Callable.Decl);
+    const BindArgument &ObjPtr = FunctionCallArgs.front();
+    assert(ObjPtr.Kind == BK_ObjectPtr);
+
+    Stream << " { ";
+    if (!llvm::isa<CXXThisExpr>(ignoreTemporariesAndPointers(ObjPtr.E))) {
+      if (ObjPtr.CaptureMode != CM_None)
+        Stream << ObjPtr.UsageIdentifier;
+      else
+        Stream << ObjPtr.SourceTokens;
+      Stream << "->";
+    }
+
+    Stream << MethodDecl->getName();
+  } else {
+    Stream << " { return ";
+    switch (LP.Callable.CaptureMode) {
+    case CM_ByValue:
+    case CM_ByRef:
+      if (LP.Callable.UsageIdentifier != LP.Callable.CaptureIdentifier) {
+        Stream << "(" << LP.Callable.UsageIdentifier << ")";
+        break;
+      }
+      LLVM_FALLTHROUGH;
+    case CM_InitExpression:
+      Stream << LP.Callable.UsageIdentifier;
+      break;
+    default:
+      Ref->printPretty(Stream, nullptr, Result.Context->getPrintingPolicy());
+    }
+  }
+
   Stream << "(";
-  addFunctionCallArgs(Args, Stream);
+
+  addFunctionCallArgs(getForwardedArgumentList(LP), Stream);
   Stream << "); }";
 
   Diag << FixItHint::CreateReplacement(MatchedDecl->getSourceRange(),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to