[clang-tools-extra] [clang-tidy] fix false positive in cppcoreguidelines-missing-std-forward (PR #77056)

[Qizhi Hu via cfe-commits]

https://github.com/jcsxky updated https://github.com/llvm/llvm-project/pull/77056

>From 043fc62485293f5cdc41ed8e4afd056671b0ea06 Mon Sep 17 00:00:00 2001
From: huqizhi <huqizhi at feysh.com>
Date: Wed, 3 Jan 2024 09:44:26 +0800
Subject: [PATCH] [clang-tidy] fix false positive in
 cppcoreguidelines-missing-std-forward

---
 .../MissingStdForwardCheck.cpp                | 72 +++++++++++++++++--
 .../cppcoreguidelines/missing-std-forward.cpp | 26 ++++++-
 2 files changed, 88 insertions(+), 10 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
index 0b85ea19735eef..ae31ce13f8def9 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
@@ -53,18 +53,76 @@ AST_MATCHER(ParmVarDecl, isTemplateTypeParameter) {
          FuncTemplate->getTemplateParameters()->getDepth();
 }
 
+AST_MATCHER_P(NamedDecl, hasSameNameAsBoundNode, std::string, BindingID) {
+  const auto &Name = Node.getNameAsString();
+
+  return Builder->removeBindings(
+      [this, Name](const ast_matchers::internal::BoundNodesMap &Nodes) {
+        const auto &BN = Nodes.getNode(this->BindingID);
+        if (const auto *ND = BN.get<NamedDecl>()) {
+          if (!isa<FieldDecl, CXXMethodDecl, VarDecl>(ND))
+            return true;
+          return ND->getName() != Name;
+        }
+        return true;
+      });
+}
+
+AST_MATCHER_P(LambdaCapture, hasCaptureKind, LambdaCaptureKind, Kind) {
+  return Node.getCaptureKind() == Kind;
+}
+
+AST_MATCHER_P(LambdaExpr, hasCaptureDefaultKind, LambdaCaptureDefault, Kind) {
+  return Node.getCaptureDefault() == Kind;
+}
+
+AST_MATCHER(LambdaExpr, hasCaptureToParm) {
+  auto RefToParm = capturesVar(varDecl(hasSameNameAsBoundNode("param")));
+  auto HasRefToParm = hasAnyCapture(RefToParm);
+
+  auto CaptureInRef =
+      allOf(hasCaptureDefaultKind(LambdaCaptureDefault::LCD_ByRef),
+            unless(HasRefToParm));
+  auto CaptureInCopy = allOf(
+      hasCaptureDefaultKind(LambdaCaptureDefault::LCD_ByCopy), HasRefToParm);
+  auto CaptureByRefExplicit = hasAnyCapture(
+      allOf(hasCaptureKind(LambdaCaptureKind::LCK_ByRef), RefToParm));
+
+  auto Captured =
+      lambdaExpr(anyOf(CaptureInRef, CaptureInCopy, CaptureByRefExplicit));
+  if (Captured.matches(Node, Finder, Builder))
+    return true;
+
+  return false;
+}
+
+AST_MATCHER(CallExpr, forCallableNode) {
+  auto InvokeInCaptureList = hasAnyCapture(capturesVar(
+      varDecl(hasInitializer(ignoringParenImpCasts(equalsNode(&Node))))));
+  auto InvokeInLambda = hasDeclContext(cxxRecordDecl(
+      isLambda(),
+      hasParent(lambdaExpr(forCallable(equalsBoundNode("func")),
+                           anyOf(InvokeInCaptureList, hasCaptureToParm())))));
+
+  if (forCallable(anyOf(equalsBoundNode("func"), InvokeInLambda))
+          .matches(Node, Finder, Builder))
+    return true;
+
+  return false;
+}
+
 } // namespace
 
 void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) {
   auto ToParam = hasAnyParameter(parmVarDecl(equalsBoundNode("param")));
 
-  auto ForwardCallMatcher = callExpr(
-      forCallable(equalsBoundNode("func")), argumentCountIs(1),
-      callee(unresolvedLookupExpr(hasAnyDeclaration(
-          namedDecl(hasUnderlyingDecl(hasName("::std::forward")))))),
-      hasArgument(0, declRefExpr(to(equalsBoundNode("param"))).bind("ref")),
-      unless(anyOf(hasAncestor(typeLoc()),
-                   hasAncestor(expr(hasUnevaluatedContext())))));
+  auto ForwardCallMatcher =
+      callExpr(forCallableNode(), argumentCountIs(1),
+               callee(unresolvedLookupExpr(hasAnyDeclaration(
+                   namedDecl(hasUnderlyingDecl(hasName("::std::forward")))))),
+               hasArgument(0, declRefExpr(to(equalsBoundNode("param")))),
+               unless(anyOf(hasAncestor(typeLoc()),
+                            hasAncestor(expr(hasUnevaluatedContext())))));
 
   Finder->addMatcher(
       parmVarDecl(parmVarDecl().bind("param"), isTemplateTypeParameter(),
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
index b9720db272e406..55d6be743c22ab 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
@@ -90,9 +90,9 @@ void lambda_value_capture(T&& t) {
 }
 
 template <class T>
-void lambda_value_reference(T&& t) {
-  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
-  [&]() { T other = std::forward<T>(t); };
+void lambda_value_capture_copy(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+  [&,t]() { T other = std::forward<T>(t); };
 }
 
 } // namespace positive_cases
@@ -147,4 +147,24 @@ class AClass {
   T data;
 };
 
+template <class T>
+void lambda_value_reference(T&& t) {
+  [&]() { T other = std::forward<T>(t); };
+}
+
+template<typename T>
+void lambda_value_reference_capture_list_ref_1(T&& t) {
+    [=, &t] { T other = std::forward<T>(t); };
+}
+
+template<typename T>
+void lambda_value_reference_capture_list_ref_2(T&& t) {
+    [&t] { T other = std::forward<T>(t); };
+}
+
+template<typename T>
+void lambda_value_reference_capture_list(T&& t) {
+    [t = std::forward<T>(t)] { t(); };
+}
+
 } // namespace negative_cases