[clang-tidy] fix false positive in cppcoreguidelines-missing-std-forw…

…ard (#77056) Parameter variable which is forwarded in lambda capture list or in body by reference is reasonable and current version of this check produces false positive on these cases. This patch try to fix the [issue](#68105) Co-authored-by: huqizhi <836744285@qq.com>
llvm · Jan 6, 2024 · d084829 · d084829
1 parent bd0dc35
commit d084829
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 5 deletions.
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
@@ -53,16 +53,72 @@ AST_MATCHER(ParmVarDecl, isTemplateTypeParameter) {
          FuncTemplate->getTemplateParameters()->getDepth();
 }
 
+AST_MATCHER_P(NamedDecl, hasSameNameAsBoundNode, std::string, BindingID) {
+  IdentifierInfo *II = Node.getIdentifier();
+  if (nullptr == II)
+    return false;
+  StringRef Name = II->getName();
+
+  return Builder->removeBindings(
+      [this, Name](const ast_matchers::internal::BoundNodesMap &Nodes) {
+        const DynTypedNode &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;
+}
+
 } // namespace
 
 void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) {
+  auto RefToParmImplicit = allOf(
+      equalsBoundNode("var"), hasInitializer(ignoringParenImpCasts(
+                                  declRefExpr(to(equalsBoundNode("param"))))));
+  auto RefToParm = capturesVar(
+      varDecl(anyOf(hasSameNameAsBoundNode("param"), RefToParmImplicit)));
+  auto HasRefToParm = hasAnyCapture(RefToParm);
+
+  auto CaptureInRef =
+      allOf(hasCaptureDefaultKind(LambdaCaptureDefault::LCD_ByRef),
+            unless(hasAnyCapture(
+                capturesVar(varDecl(hasSameNameAsBoundNode("param"))))));
+  auto CaptureInCopy = allOf(
+      hasCaptureDefaultKind(LambdaCaptureDefault::LCD_ByCopy), HasRefToParm);
+  auto CaptureByRefExplicit = hasAnyCapture(
+      allOf(hasCaptureKind(LambdaCaptureKind::LCK_ByRef), RefToParm));
+
+  auto CapturedInBody =
+      lambdaExpr(anyOf(CaptureInRef, CaptureInCopy, CaptureByRefExplicit));
+  auto CapturedInCaptureList = hasAnyCapture(capturesVar(
+      varDecl(hasInitializer(ignoringParenImpCasts(equalsBoundNode("call"))))));
+
+  auto CapturedInLambda = hasDeclContext(cxxRecordDecl(
+      isLambda(),
+      hasParent(lambdaExpr(forCallable(equalsBoundNode("func")),
+                           anyOf(CapturedInCaptureList, CapturedInBody)))));
+
   auto ToParam = hasAnyParameter(parmVarDecl(equalsBoundNode("param")));
 
   auto ForwardCallMatcher = callExpr(
-      forCallable(equalsBoundNode("func")), argumentCountIs(1),
+      callExpr().bind("call"), argumentCountIs(1),
+      hasArgument(
+          0, declRefExpr(to(
+                 varDecl(optionally(equalsBoundNode("param"))).bind("var")))),
+      forCallable(anyOf(equalsBoundNode("func"), CapturedInLambda)),
       callee(unresolvedLookupExpr(hasAnyDeclaration(
           namedDecl(hasUnderlyingDecl(hasName("::std::forward")))))),
-      hasArgument(0, declRefExpr(to(equalsBoundNode("param"))).bind("ref")),
+
       unless(anyOf(hasAncestor(typeLoc()),
                    hasAncestor(expr(hasUnevaluatedContext())))));
 

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -301,6 +301,10 @@ Changes in existing checks
   coroutine functions and increase issue detection for cases involving type
   aliases with references.
 
+- Improved :doc:`cppcoreguidelines-missing-std-forward
+  <clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check to
+  address false positives in the capture list and body of lambdas.
+
 - Improved :doc:`cppcoreguidelines-narrowing-conversions
   <clang-tidy/checks/cppcoreguidelines/narrowing-conversions>` check by
   extending the `IgnoreConversionFromTypes` option to include types without a

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
@@ -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,29 @@ 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(); };
+}
+
+template <class T>
+void lambda_value_reference_auxiliary_var(T&& t) {
+  [&x = t]() { T other = std::forward<T>(x); };
+}
+
 } // namespace negative_cases