[clang-tools-extra] [clang-tidy] modernize-avoid-bind only return for non-void function (PR #69207)

via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 16 12:58:30 PDT 2023


https://github.com/5chmidti updated https://github.com/llvm/llvm-project/pull/69207

>From 7771acd00ace1362e580531f1a3ed63f81cfa5a3 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <44101708+5chmidti at users.noreply.github.com>
Date: Mon, 14 Aug 2023 03:04:36 +0200
Subject: [PATCH 1/5] [clang-tidy] modernize-avoid-bind only return for
 non-void function

- only return when the return type is non-void
- fix missing return on member functions
---
 .../clang-tidy/modernize/AvoidBindCheck.cpp   | 13 ++++--
 .../checkers/modernize/avoid-bind.cpp         | 43 +++++++++++++------
 2 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp b/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
index 4628c09f196996b..bcdf6f98055486e 100644
--- a/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
@@ -96,6 +96,7 @@ struct CallableInfo {
   std::string UsageIdentifier;
   StringRef CaptureInitializer;
   const FunctionDecl *Decl = nullptr;
+  bool DoesReturn = false;
 };
 
 struct LambdaProperties {
@@ -554,6 +555,8 @@ getLambdaProperties(const MatchFinder::MatchResult &Result) {
   LP.Callable.Materialization = getCallableMaterialization(Result);
   LP.Callable.Decl =
       getCallMethodDecl(Result, LP.Callable.Type, LP.Callable.Materialization);
+  if (LP.Callable.Decl)
+    LP.Callable.DoesReturn = !LP.Callable.Decl->getReturnType()->isVoidType();
   LP.Callable.SourceTokens = getSourceTextForExpr(Result, CalleeExpr);
   if (LP.Callable.Materialization == CMK_VariableRef) {
     LP.Callable.CE = CE_Var;
@@ -672,15 +675,20 @@ void AvoidBindCheck::check(const MatchFinder::MatchResult &Result) {
 
   addPlaceholderArgs(LP, Stream, PermissiveParameterList);
 
+  Stream << " { ";
+
+  if (LP.Callable.DoesReturn) {
+    Stream << "return ";
+  }
+
   if (LP.Callable.Type == CT_Function) {
     StringRef SourceTokens = LP.Callable.SourceTokens;
     SourceTokens.consume_front("&");
-    Stream << " { return " << SourceTokens;
+    Stream << SourceTokens;
   } else if (LP.Callable.Type == CT_MemberFunction) {
     const auto *MethodDecl = dyn_cast<CXXMethodDecl>(LP.Callable.Decl);
     const BindArgument &ObjPtr = FunctionCallArgs.front();
 
-    Stream << " { ";
     if (!isa<CXXThisExpr>(ignoreTemporariesAndPointers(ObjPtr.E))) {
       Stream << ObjPtr.UsageIdentifier;
       Stream << "->";
@@ -688,7 +696,6 @@ void AvoidBindCheck::check(const MatchFinder::MatchResult &Result) {
 
     Stream << MethodDecl->getName();
   } else {
-    Stream << " { return ";
     switch (LP.Callable.CE) {
     case CE_Var:
       if (LP.Callable.UsageIdentifier != LP.Callable.CaptureIdentifier) {
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
index 3334eab2d407e49..336b3cb4ee08f94 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
@@ -45,6 +45,7 @@ struct D {
   void operator()(int x, int y) const {}
 
   void MemberFunction(int x) {}
+  int MemberFunctionWithReturn(int x) {}
 
   static D *create();
 };
@@ -162,11 +163,11 @@ struct TestCaptureByValueStruct {
     // 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); };
+    // CHECK-FIXES: auto FFF = [f] { 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); };
+    // CHECK-FIXES: auto GGG = [this] { UseF(MemberStruct); };
 
     auto HHH = std::bind(add, MemberStructWithData._member, 1);
     // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
@@ -227,34 +228,34 @@ void testFunctionObjects() {
   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); }
+  // CHECK-FIXES: auto AAA = [d] { 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); }
+  // CHECK-FIXES: auto BBB = [e] { (*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); }
+  // CHECK-FIXES: auto CCC = [] { 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); }
+  // CHECK-FIXES: auto DDD = [] { 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); };
+  // CHECK-FIXES: auto EEE = [Func = *D::create()] { 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); };
+  // CHECK-FIXES: auto FFF = [] { 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); };
+  // CHECK-FIXES: auto GGG = [Func = G(CTorArg)] { Func(1); };
 }
 
 template <typename T>
@@ -262,7 +263,7 @@ 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); };
+  // CHECK-FIXES: auto HHH = [] { H<T>()(42); };
 }
 
 template void testMemberFnOfClassTemplate(int);
@@ -335,11 +336,12 @@ void testCapturedSubexpressions() {
   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)); };
+  // CHECK-FIXES: auto CCC = [&capture0 = *p](auto && PH1) { sub(capture0, std::forward<decltype(PH1)>(PH1)); };
 }
 
 struct E {
   void MemberFunction(int x) {}
+  int MemberFunctionWithReturn(int x) {}
 
   void testMemberFunctions() {
     D *d;
@@ -360,6 +362,23 @@ struct E {
     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::MemberFunctionWithReturn, d, 1);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+    // CHECK-FIXES: auto EEE = [d] { return d->MemberFunctionWithReturn(1); };
+
+    auto FFF = std::bind(&D::MemberFunctionWithReturn, &dd, 1);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+    // CHECK-FIXES: auto FFF = [ObjectPtr = &dd] { return ObjectPtr->MemberFunctionWithReturn(1); };
+
+    auto GGG = std::bind(&E::MemberFunctionWithReturn, this, 1);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+    // CHECK-FIXES: auto GGG = [this] { return MemberFunctionWithReturn(1); };
+
+    // Test what happens when the object pointer is itself a placeholder.
+    auto HHH = std::bind(&D::MemberFunctionWithReturn, _1, 1);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+    // CHECK-FIXES: auto HHH = [](auto && PH1) { return PH1->MemberFunctionWithReturn(1); };
   }
 };
 
@@ -368,5 +387,5 @@ void testStdFunction(Thing *t) {
   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); });
+  // CHECK-FIXES: cb.Reset([t] { UseThing(t); });
 }

>From 8225a74a2c636f5ea161446ceb41af2f08a5855d Mon Sep 17 00:00:00 2001
From: Julian Schmidt <44101708+5chmidti at users.noreply.github.com>
Date: Mon, 16 Oct 2023 17:17:19 +0200
Subject: [PATCH 2/5] address comments

---
 clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp | 3 ++-
 clang-tools-extra/docs/ReleaseNotes.rst                   | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp b/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
index bcdf6f98055486e..fb83e757b27dbdf 100644
--- a/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
@@ -556,7 +556,8 @@ getLambdaProperties(const MatchFinder::MatchResult &Result) {
   LP.Callable.Decl =
       getCallMethodDecl(Result, LP.Callable.Type, LP.Callable.Materialization);
   if (LP.Callable.Decl)
-    LP.Callable.DoesReturn = !LP.Callable.Decl->getReturnType()->isVoidType();
+    if (const Type *ReturnType = LP.Callable.Decl->getReturnType().getTypePtr())
+      LP.Callable.DoesReturn = !ReturnType->isVoidType();
   LP.Callable.SourceTokens = getSourceTextForExpr(Result, CalleeExpr);
   if (LP.Callable.Materialization == CMK_VariableRef) {
     LP.Callable.CE = CE_Var;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index af164d0462d52c1..d9263c879758a91 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -122,6 +122,8 @@ Improvements to clang-tidy
   if any :program:`clang-tidy` subprocess exits with a non-zero code or if
   exporting fixes fails.
 
+- Do not emit a `return` for fixes of `modernize-avoid-bind` when the function returns void.
+
 New checks
 ^^^^^^^^^^
 

>From 3b770ddf8fcb51ac28311f235c24f1de872d2bed Mon Sep 17 00:00:00 2001
From: Julian Schmidt <44101708+5chmidti at users.noreply.github.com>
Date: Mon, 16 Oct 2023 20:42:18 +0200
Subject: [PATCH 3/5] fix release note

---
 clang-tools-extra/docs/ReleaseNotes.rst | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index d9263c879758a91..40ca4a801ff50ef 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -122,8 +122,6 @@ Improvements to clang-tidy
   if any :program:`clang-tidy` subprocess exits with a non-zero code or if
   exporting fixes fails.
 
-- Do not emit a `return` for fixes of `modernize-avoid-bind` when the function returns void.
-
 New checks
 ^^^^^^^^^^
 
@@ -345,6 +343,10 @@ Changes in existing checks
   <clang-tidy/checks/readability/static-accessed-through-instance>` check to
   identify calls to static member functions with out-of-class inline definitions.
 
+- Improved :doc:`modernize-avoid-bind
+  <clang-tidy/checks/modernize/avoid-bind>` check to
+  not emit a `return` for fixes when the function returns void.
+
 Removed checks
 ^^^^^^^^^^^^^^
 

>From 14972fc11028595ec3e21ddad0322c08ad57e073 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <44101708+5chmidti at users.noreply.github.com>
Date: Mon, 16 Oct 2023 20:42:42 +0200
Subject: [PATCH 4/5] fix missing getCanonicalType

---
 clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp b/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
index fb83e757b27dbdf..2a54eaddccb15cf 100644
--- a/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
@@ -556,7 +556,8 @@ getLambdaProperties(const MatchFinder::MatchResult &Result) {
   LP.Callable.Decl =
       getCallMethodDecl(Result, LP.Callable.Type, LP.Callable.Materialization);
   if (LP.Callable.Decl)
-    if (const Type *ReturnType = LP.Callable.Decl->getReturnType().getTypePtr())
+    if (const Type *ReturnType =
+            LP.Callable.Decl->getReturnType().getCanonicalType().getTypePtr())
       LP.Callable.DoesReturn = !ReturnType->isVoidType();
   LP.Callable.SourceTokens = getSourceTextForExpr(Result, CalleeExpr);
   if (LP.Callable.Materialization == CMK_VariableRef) {

>From 5f4951aa0b5386ca8f6e3865bde73491e412004a Mon Sep 17 00:00:00 2001
From: Julian Schmidt <44101708+5chmidti at users.noreply.github.com>
Date: Mon, 16 Oct 2023 21:57:34 +0200
Subject: [PATCH 5/5] address release notes comment

---
 clang-tools-extra/docs/ReleaseNotes.rst | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 40ca4a801ff50ef..08fd07f83952d32 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -269,6 +269,10 @@ Changes in existing checks
   <clang-tidy/checks/misc/redundant-expression>` check to ignore
   false-positives in unevaluated context (e.g., ``decltype``).
 
+- Improved :doc:`modernize-avoid-bind
+  <clang-tidy/checks/modernize/avoid-bind>` check to
+  not emit a ``return`` for fixes when the function returns ``void``.
+
 - Improved :doc:`modernize-loop-convert
   <clang-tidy/checks/modernize/loop-convert>` to support for-loops with
   iterators initialized by free functions like ``begin``, ``end``, or ``size``
@@ -343,10 +347,6 @@ Changes in existing checks
   <clang-tidy/checks/readability/static-accessed-through-instance>` check to
   identify calls to static member functions with out-of-class inline definitions.
 
-- Improved :doc:`modernize-avoid-bind
-  <clang-tidy/checks/modernize/avoid-bind>` check to
-  not emit a `return` for fixes when the function returns void.
-
 Removed checks
 ^^^^^^^^^^^^^^
 



More information about the cfe-commits mailing list