[clang-tidy] modernize-avoid-bind: Fix crash when method name is not a simple identifier

[jspam]

Summary

The following assertion fails when referencing an operator member:

include/clang/AST/Decl.h:275: llvm::StringRef clang::NamedDecl::getName() const: Assertion `Name.isIdentifier() && "Name is not a simple identifier"' failed.

Additionally, use the call syntax instead of "operator()" and fix
missing "return" keyword.

[njames93]

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

367

Fixing a crash is good, but these fixes leave a lot to be desired. If you could handle the cases when the name is an operator call by actually using the operator it would be be a lot nicer.

auto EEE = [d] { d(1, 2); };

This definitely could be done in a follow up commit.

[jspam]

Improve fixes

As suggested by njames93, use call syntax instead of operator().

[PiotrZSL]

Crash no longer happens on main, but this change introduce nice fix for operators handling.

[PiotrZSL]

Rebase

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

if (LP.Callable.Type == CT_Function) {
     const BindArgument &ObjPtr = FunctionCallArgs.front();
 
     Stream << " { ";
     if (!isa<CXXThisExpr>(ignoreTemporariesAndPointers(ObjPtr.E))) {
       Stream << ObjPtr.UsageIdentifier;
       Stream << "->";
     }
 
-    Stream << MethodDecl->getName();
+    Stream << MethodDecl->getNameAsString();
   } else {
     Stream << " { return ";
     switch (LP.Callable.CE) {
     case CE_Var:
       if (LP.Callable.UsageIdentifier != LP.Callable.CaptureIdentifier) {
         Stream << "(" << LP.Callable.UsageIdentifier << ")";
         break;
       }

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

 
 struct Foo {
   static int add(int x, int y) { return x + y; }
 };
 
 struct D {
   D() = default;
   void operator()(int x, int y) const {}
+  operator bool() const { return true; }
 
   void MemberFunction(int x) {}
 
   static D *create();
 };
 
 struct F {
   F(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); };
 
     // Test what happens when the object pointer is itself a placeholder.
     auto DDD = std::bind(&D::MemberFunction, _1, 1);
     // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
     // CHECK-FIXES: auto DDD = [](auto && PH1) { PH1->MemberFunction(1); };
+
+    auto EEE = std::bind(&D::operator(), d, 1, 2);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+    // CHECK-FIXES: auto EEE = [d] { d->operator()(1, 2); }
+
+    auto FFF = std::bind(&D::operator bool, d);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+    // CHECK-FIXES: auto FFF = [d] { d->operator bool(); }
   }
 };
 
 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([t] { return UseThing(t); });
 }