[clang-tidy] Repair various issues with modernize-avoid-bind

[jaafar]

Summary

In the process of running this check on a large codebase I found a number of limitations, and thought I would pass on my fixes for possible integration upstream:

1. Templated function call operators are not supported
2. Function object constructors are always used directly in the lambda body, even if their arguments are not captured
3. Placeholders with namespace qualifiers (std::placeholders::_1) are not detected
4. Lambda arguments should be forwarded to the stored function
5. Data members from other classes still get captured with this
6. Expressions (as opposed to variables) inside std::ref are not captured properly
7. Function object templates sometimes have their template arguments replaced with concrete types

This patch resolves all those issues and adds suitable unit tests.

If desired, I can separate these commits out into 7 separate diffs, but it seemed like it might be easier to evaluate them all at once.

[jaafar]

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

38–39

"Captured by reference" and "uses init expression" are not mutually exclusive - that gave rise to item 6 above.

156

These forward declarations seemed to make the diffs a lot easier to read. It's also possible to move the two functions into this position, of course.

244

Is there a better way to express this? "a single child of type ThisExpr" was what I was going for...

[jaafar]

"Ping". I hope this can be considered for 10.0.1. If nothing else I think reviewing the test cases has a lot of value - there are some real issues with the current checks and fixits.

[aaron.ballman]

In D81769#2106574, @jaafar wrote:

"Ping". I hope this can be considered for 10.0.1. If nothing else I think reviewing the test cases has a lot of value - there are some real issues with the current checks and fixits.

Thank you for all of these fixes! I like the direction it's going in, but am a bit wary of introducing this for 10.0.1 because it changes a lot of behaviors (not to say it shouldn't be 10.0.1, just to express a bit of trepidation).

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

156

I think it's fine to use forward declares, but I'd prefer them just after the anonymous namespace (or before it) so the declarations are towards the top of the file instead of stuck in the middle.

167

Comments should be grammatical, so include capitalization and punctuation. (Here and elsewhere in the patch.)

244

if (isa<CXXThisExpr>(ME->getBase())) -- MemberExpr::children() only considers the base anyway, so there's only ever one child node to begin with.

282–285

const auto * since the type is spelled out in the initialization.

414–418

This doesn't need to be done in two steps. Better:

const auto *FTD = dyn_cast<FunctionTemplateDecl>(D);
if (!FTD)
  continue;

const FunctionDecl *FD = FTD->getTemplatedDecl();

[jaafar]

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

244

Could there be zero? I'm just worried about dereferencing ME->children().begin() before verifying there is at least one...

[jaafar]

Applied feedback from Aaron Ballman. Thanks!

[njames93]

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

244

The child nodes in MemberExpr just point to one node, its base, which should never be null. Is it not simpler to just use this?

if (isa<CXXThisExpr>(ME->getBase())) {

[jaafar]

Applied feedback from Nathan James

[jaafar]

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

244

Ah, much better, thanks!

[aaron.ballman]

Aside from a minor nit, this LGTM

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

414–415

You can drop this entirely -- the dyn_cast<> below does the work for you.

[jaafar]

One more simplification from Aaron. Thanks!

[aaron.ballman]

In D81769#2109936, @jaafar wrote:

One more simplification from Aaron. Thanks!

No problem! Do you need someone to commit on your behalf, btw? If so, let me know what name and email address you would like me to attribute.

[jaafar]

I do need someone to commit this, yes, thank you.

I'm Jeff Trull [e-mail]

[aaron.ballman]

Thank you for the patch! I've commit on your behalf in 95a3550dc89a0d424d90e2c0ad30d9ecfa9422cf

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

 
 namespace clang {
 namespace tidy {
 namespace modernize {
 
 namespace {
 
 enum BindArgumentKind { BK_Temporary, BK_Placeholder, BK_CallExpr, BK_Other };
-enum CaptureMode { CM_None, CM_ByRef, CM_ByValue, CM_InitExpression };
+enum CaptureMode { CM_None, CM_ByRef, CM_ByValue };
+enum CaptureExpr { CE_None, CE_Var, CE_InitExpression };
 
 enum CallableType {
   CT_Other,          // unknown
   CT_Function,       // global or static function
   CT_MemberFunction, // member function with implicit this
   CT_Object,         // object with operator()
 };
 
 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 {
   // 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 CM = CM_None;
 
+  // Whether the argument is a simple variable (we can capture it directly),
+  // or an expression (we must introduce a capture variable).
+  CaptureExpr CE = CE_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;
 
struct BindArgument {
   // The actual Expr object representing this expression.
   const Expr *E = nullptr;
 };
 
 struct CallableInfo {
   CallableType Type = CT_Other;
   CallableMaterializationKind Materialization = CMK_Other;
   CaptureMode CM = CM_None;
+  CaptureExpr CE = CE_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 bool tryCaptureAsLocalVariable(const MatchFinder::MatchResult &Result,
+                                      BindArgument &B, const Expr *E);
+
+static bool tryCaptureAsMemberVariable(const MatchFinder::MatchResult &Result,
+                                       BindArgument &B, const Expr *E);
+
 static const Expr *ignoreTemporariesAndPointers(const Expr *E) {
   if (const auto *T = dyn_cast<UnaryOperator>(E))
     return ignoreTemporariesAndPointers(T->getSubExpr());
 
   const Expr *F = E->IgnoreImplicit();
   if (E != F)
     return ignoreTemporariesAndPointers(F);
 
static bool isCallExprNamed(const Expr *E, StringRef Name) {
   if (!CE)
     return false;
   const auto *ND = dyn_cast<NamedDecl>(CE->getCalleeDecl());
   if (!ND)
     return false;
   return ND->getQualifiedNameAsString() == Name;
 }
 
 static void
 initializeBindArgumentForCallExpr(const MatchFinder::MatchResult &Result,
                                   BindArgument &B, const CallExpr *CE,
                                   unsigned &CaptureIndex) {
   // std::ref(x) means to capture x by reference.
   if (isCallExprNamed(CE, "boost::ref") || isCallExprNamed(CE, "std::ref")) {
     B.Kind = BK_Other;
+    if (tryCaptureAsLocalVariable(Result, B, CE->getArg(0)) ||
+        tryCaptureAsMemberVariable(Result, B, CE->getArg(0))) {
+      B.CE = CE_Var;
+    } else {
+      // The argument to std::ref is an expression that produces a reference.
+      // Create a capture reference to hold it.
+      B.CE = CE_InitExpression;
+      B.UsageIdentifier = "capture" + llvm::utostr(CaptureIndex++);
+    }
+    // Strip off the reference wrapper.
+    B.SourceTokens = getSourceTextForExpr(Result, CE->getArg(0));
     B.CM = CM_ByRef;
-    B.UsageIdentifier =
-        std::string(getSourceTextForExpr(Result, CE->getArg(0)));
   } else {
     B.Kind = BK_CallExpr;
-    B.CM = CM_InitExpression;
+    B.CM = CM_ByValue;
+    B.CE = CE_InitExpression;
     B.UsageIdentifier = "capture" + llvm::utostr(CaptureIndex++);
   }
   B.CaptureIdentifier = B.UsageIdentifier;
 }
 
 static bool anyDescendantIsLocal(const Stmt *Statement) {
   if (const auto *DeclRef = dyn_cast<DeclRefExpr>(Statement)) {
     const ValueDecl *Decl = DeclRef->getDecl();
static bool tryCaptureAsMemberVariable(const MatchFinder::MatchResult &Result,
   if (const auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E)) {
     if (const auto *CE = dyn_cast<CXXConstructExpr>(BTE->getSubExpr()))
       return tryCaptureAsMemberVariable(Result, B, CE->getArg(0));
     return false;
   }
 
   E = E->IgnoreImplicit();
   if (isa<CXXThisExpr>(E)) {
+    // E is a direct use of "this".
     B.CM = CM_ByValue;
     B.UsageIdentifier = std::string(getSourceTextForExpr(Result, E));
     B.CaptureIdentifier = "this";
     return true;
   }
 
   const auto *ME = dyn_cast<MemberExpr>(E);
   if (!ME)
     return false;
 
   if (!ME->isLValue() || !isa<FieldDecl>(ME->getMemberDecl()))
     return false;
 
+  if (isa<CXXThisExpr>(ME->getBase())) {
+    // E refers to a data member without an explicit "this".
     B.CM = CM_ByValue;
     B.UsageIdentifier = std::string(getSourceTextForExpr(Result, E));
     B.CaptureIdentifier = "this";
     return true;
   }
 
+  return false;
+}
+
 static SmallVector<BindArgument, 4>
 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");
 
for (size_t I = 1, ArgCount = BindCall->getNumArgs(); I < ArgCount; ++I) {
     B.E = E;
     B.SourceTokens = getSourceTextForExpr(Result, E);
 
     if (!Callable.Decl || ArgIndex < Callable.Decl->getNumParams() ||
         IsObjectPtr)
       B.IsUsed = true;
 
     SmallVector<StringRef, 2> Matches;
-    if (MatchPlaceholder.match(B.SourceTokens, &Matches)) {
+    const auto *DRE = dyn_cast<DeclRefExpr>(E);
+    if (MatchPlaceholder.match(B.SourceTokens, &Matches) ||
+        // Check for match with qualifiers removed.
+        (DRE && MatchPlaceholder.match(DRE->getDecl()->getName(), &Matches))) {
       B.Kind = BK_Placeholder;
       B.PlaceHolderIndex = std::stoi(std::string(Matches[1]));
       B.UsageIdentifier = "PH" + llvm::utostr(B.PlaceHolderIndex);
       B.CaptureIdentifier = B.UsageIdentifier;
       continue;
     }
 
     if (const auto *CE =
             dyn_cast<CallExpr>(ignoreTemporariesAndConstructors(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 (IsObjectPtr) {
-      B.CM = CM_InitExpression;
+      B.CE = CE_InitExpression;
+      B.CM = CM_ByValue;
       B.UsageIdentifier = "ObjectPtr";
       B.CaptureIdentifier = B.UsageIdentifier;
     } else if (anyDescendantIsLocal(B.E)) {
-      B.CM = CM_InitExpression;
+      B.CE = CE_InitExpression;
+      B.CM = CM_ByValue;
       B.CaptureIdentifier = "capture" + llvm::utostr(CaptureIndex++);
       B.UsageIdentifier = B.CaptureIdentifier;
     }
   }
   return BindArguments;
 }
 
 static int findPositionOfPlaceholderUse(ArrayRef<BindArgument> Args,
static void addFunctionCallArgs(ArrayRef<BindArgument> Args,
                                 llvm::raw_ostream &Stream) {
   StringRef Delimiter = "";
 
   for (int I = 0, Size = Args.size(); I < Size; ++I) {
     const BindArgument &B = Args[I];
 
     Stream << Delimiter;
 
-    if (B.Kind == BK_Placeholder || B.CM != CM_None)
+    if (B.Kind == BK_Placeholder) {
+      Stream << "std::forward<decltype(" << B.UsageIdentifier << ")>";
+      Stream << "(" << B.UsageIdentifier << ")";
+    } else if (B.CM != CM_None)
       Stream << B.UsageIdentifier;
-    else if (B.CM == CM_None)
+    else
       Stream << B.SourceTokens;
 
     Delimiter = ", ";
   }
 }
 
 static bool isPlaceHolderIndexRepeated(const ArrayRef<BindArgument> Args) {
   llvm::SmallSet<size_t, 4> PlaceHolderIndices;
   for (const BindArgument &B : Args) {
     if (B.PlaceHolderIndex) {
       if (!PlaceHolderIndices.insert(B.PlaceHolderIndex).second)
         return true;
     }
   }
   return false;
 }
 
-static std::vector<const CXXMethodDecl *>
+static std::vector<const FunctionDecl *>
 findCandidateCallOperators(const CXXRecordDecl *RecordDecl, size_t NumArgs) {
-  std::vector<const CXXMethodDecl *> Candidates;
+  std::vector<const FunctionDecl *> 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);
   }
 
+  // Find templated operator(), if any.
+  for (const clang::Decl *D : RecordDecl->decls()) {
+    const auto *FTD = dyn_cast<FunctionTemplateDecl>(D);
+    if (!FTD)
+      continue;
+    const FunctionDecl *FD = FTD->getTemplatedDecl();
+
+    OverloadedOperatorKind OOK = FD->getOverloadedOperator();
+    if (OOK != OverloadedOperatorKind::OO_Call)
+      continue;
+
+    if (FD->getNumParams() > NumArgs)
+      continue;
+
+    Candidates.push_back(FD);
+  }
+
   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
static bool isFixitSupported(const CallableInfo &Callee,
   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 =
+  std::vector<const FunctionDecl *> Candidates =
       findCandidateCallOperators(Callable, NumArgs);
   if (Candidates.size() != 1)
     return nullptr;
 
   return Candidates.front();
 }
 
 const FunctionDecl *

 }
 
 static CallableMaterializationKind
 getCallableMaterialization(const MatchFinder::MatchResult &Result) {
   const auto *CallableExpr = Result.Nodes.getNodeAs<Expr>("ref");
 
   const auto *NoTemporaries = ignoreTemporariesAndPointers(CallableExpr);
 
-  if (isa<CallExpr>(NoTemporaries))
+  const auto *CE = dyn_cast<CXXConstructExpr>(NoTemporaries);
+  const auto *FC = dyn_cast<CXXFunctionalCastExpr>(NoTemporaries);
+  if ((isa<CallExpr>(NoTemporaries)) || (CE && (CE->getNumArgs() > 0)) ||
+      (FC && (FC->getCastKind() == CK_ConstructorConversion)))
+    // CE is something that looks like a call, with arguments - either
+    // a function call or a constructor invocation.
     return CMK_CallExpression;
 
-  if (isa<CXXFunctionalCastExpr>(NoTemporaries) ||
-      isa<CXXConstructExpr>(NoTemporaries))
+  if (isa<CXXFunctionalCastExpr>(NoTemporaries) || CE)
     return CMK_Function;
 
   if (const auto *DRE = dyn_cast<DeclRefExpr>(NoTemporaries)) {
     if (isa<FunctionDecl>(DRE->getDecl()))
       return CMK_Function;
     if (isa<VarDecl>(DRE->getDecl()))
       return CMK_VariableRef;
   }
getLambdaProperties(const MatchFinder::MatchResult &Result) {
   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.CE = CE_Var;
     LP.Callable.CM = CM_ByValue;
     LP.Callable.UsageIdentifier =
         std::string(getSourceTextForExpr(Result, CalleeExpr));
     LP.Callable.CaptureIdentifier = std::string(
         getSourceTextForExpr(Result, ignoreTemporariesAndPointers(CalleeExpr)));
   } else if (LP.Callable.Materialization == CMK_CallExpression) {
-    LP.Callable.CM = CM_InitExpression;
+    LP.Callable.CE = CE_InitExpression;
+    LP.Callable.CM = CM_ByValue;
     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,
+                        CaptureMode CM, CaptureExpr CE, 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)
+  if (CE == CE_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.CM,
-                                   LP.Callable.CaptureIdentifier,
-                                   LP.Callable.CaptureInitializer, Stream);
+  AnyCapturesEmitted = emitCapture(
+      CaptureSet, "", LP.Callable.CM, LP.Callable.CE,
+      LP.Callable.CaptureIdentifier, LP.Callable.CaptureInitializer, Stream);
 
   for (const BindArgument &B : LP.BindArguments) {
     if (B.CM == CM_None || !B.IsUsed)
       continue;
 
     StringRef Delimiter = AnyCapturesEmitted ? ", " : "";
 
-    if (emitCapture(CaptureSet, Delimiter, B.CM, B.CaptureIdentifier,
+    if (emitCapture(CaptureSet, Delimiter, B.CM, B.CE, B.CaptureIdentifier,
                     B.SourceTokens, Stream))
       AnyCapturesEmitted = true;
   }
 }
 
 static ArrayRef<BindArgument>
 getForwardedArgumentList(const LambdaProperties &P) {
   ArrayRef<BindArgument> Args = makeArrayRef(P.BindArguments);
if (LP.Callable.Type == CT_Function) {
     if (!isa<CXXThisExpr>(ignoreTemporariesAndPointers(ObjPtr.E))) {
       Stream << ObjPtr.UsageIdentifier;
       Stream << "->";
     }
 
     Stream << MethodDecl->getName();
   } else {
     Stream << " { return ";
-    switch (LP.Callable.CM) {
-    case CM_ByValue:
-    case CM_ByRef:
+    switch (LP.Callable.CE) {
+    case CE_Var:
       if (LP.Callable.UsageIdentifier != LP.Callable.CaptureIdentifier) {
         Stream << "(" << LP.Callable.UsageIdentifier << ")";
         break;
       }
       LLVM_FALLTHROUGH;
-    case CM_InitExpression:
+    case CE_InitExpression:
       Stream << LP.Callable.UsageIdentifier;
       break;
     default:
-      Ref->printPretty(Stream, nullptr, Result.Context->getPrintingPolicy());
+      Stream << getSourceTextForExpr(Result, Ref);
     }
   }
 
   Stream << "(";
 
   addFunctionCallArgs(getForwardedArgumentList(LP), 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-permissive-parameter-list.cpp

 
 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 = [](auto && ...) { return add(2, 2); };
 
   auto BBB = std::bind(add, _1, 2);
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
-  // CHECK-FIXES: auto BBB = [](auto && PH1, auto && ...) { return add(PH1, 2); };
+  // CHECK-FIXES: auto BBB = [](auto && PH1, auto && ...) { return add(std::forward<decltype(PH1)>(PH1), 2); };
 }

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

 // RUN: %check_clang_tidy -std=c++14-or-later %s modernize-avoid-bind %t
 
 namespace std {
 inline namespace impl {
 template <class Fp, class... Arguments>
 class bind_rt {};
 
 template <class Fp, class... Arguments>
 bind_rt<Fp, Arguments...> bind(Fp &&, Arguments &&...);
-}
+} // namespace impl
 
 template <typename T>
 T ref(T &t);
-}
+} // namespace std
 
 namespace boost {
 template <class Fp, class... Arguments>
 class bind_rt {};
 
 template <class Fp, class... Arguments>
 bind_rt<Fp, Arguments...> bind(const Fp &, Arguments...);
 
struct F {
   F(int x) {}
   ~F() {}
 
   int get() { return 42; }
 };
 
 void UseF(F);
 
+struct G {
+  G() : _member(0) {}
+  G(int m) : _member(m) {}
+
+  template <typename T>
+  void operator()(T) const {}
+
+  int _member;
+};
+
+template <typename T>
+struct H {
+  void operator()(T) const {};
+};
+
 struct placeholder {};
 placeholder _1;
 placeholder _2;
 
+namespace placeholders {
+using ::_1;
+using ::_2;
+} // namespace placeholders
+
 int add(int x, int y) { return x + y; }
 int addThree(int x, int y, int z) { return x + y + z; }
+void sub(int &x, int y) { x += y; }
 
 // Let's fake a minimal std::function-like facility.
 namespace std {
 template <typename _Tp>
 _Tp declval();
 
 template <typename _Functor, typename... _ArgTypes>
 struct __res {

 };
 
 int GlobalVariable = 42;
 
 struct TestCaptureByValueStruct {
   int MemberVariable;
   static int StaticMemberVariable;
   F MemberStruct;
+  G MemberStructWithData;
 
   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); };
 
void testCaptureByValue(int Param, F f) {
     // 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); };
+
+    auto HHH = std::bind(add, MemberStructWithData._member, 1);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+    // Correctly distinguish data members of other classes
+    // CHECK-FIXES: auto HHH = [capture0 = MemberStructWithData._member] { return add(capture0, 1); };
   }
 };
 
 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); };
 
void testFunctionObjects() {
 
   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); };
+
+  auto FFF = std::bind(G(), 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // Templated function call operators may be used
+  // CHECK-FIXES: auto FFF = [] { return G()(1); };
+
+  int CTorArg = 42;
+  auto GGG = std::bind(G(CTorArg), 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // Function objects with constructor arguments should be captured
+  // CHECK-FIXES: auto GGG = [Func = G(CTorArg)] { return Func(1); };
 }
 
+template <typename T>
+void testMemberFnOfClassTemplate(T) {
+  auto HHH = std::bind(H<T>(), 42);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // Ensure function class template arguments are preserved
+  // CHECK-FIXES: auto HHH = [] { return H<T>()(42); };
+}
+
+template void testMemberFnOfClassTemplate(int);
+
 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); };
+  // CHECK-FIXES: auto AAA = [x](auto && PH1) { return add(x, std::forward<decltype(PH1)>(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); };
+  // CHECK-FIXES: auto BBB = [](auto && PH1, auto && PH2) { return add(std::forward<decltype(PH2)>(PH2), std::forward<decltype(PH1)>(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);
 
   // When a placeholder is skipped, we always add skipped ones to the lambda as
   // unnamed parameters.
   auto DDD = std::bind(add, _2, 1);
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto DDD = [](auto &&, auto && PH2) { return add(PH2, 1); };
+  // CHECK-FIXES: auto DDD = [](auto &&, auto && PH2) { return add(std::forward<decltype(PH2)>(PH2), 1); };
+
+  // Namespace-qualified placeholders are valid too
+  auto EEE = std::bind(add, placeholders::_2, 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // CHECK-FIXES: auto EEE = [](auto &&, auto && PH2) { return add(std::forward<decltype(PH2)>(PH2), 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);
void testGlobalFunctions() {
   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;
+  int *p = &x;
 
   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 = [capture0 = add(2, 5)] { return add(1, capture0); };
 
   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, capture0 = add(y, 5)] { return add(x, capture0); };
+
+  auto CCC = std::bind(sub, std::ref(*p), _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // Expressions returning references are captured
+  // CHECK-FIXES: auto CCC = [&capture0 = *p](auto && PH1) { return sub(capture0, std::forward<decltype(PH1)>(PH1)); };
 }
 
 struct E {
   void MemberFunction(int x) {}
 
   void testMemberFunctions() {
     D *d;
     D dd;