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

[via cfe-commits]

Author: Qizhi Hu
Date: 2024-01-06T19:50:00+08:00
New Revision: d08482924efe8b2c44913583af7b8f60a29975d1

URL: https://github.com/llvm/llvm-project/commit/d08482924efe8b2c44913583af7b8f60a29975d1
DIFF: https://github.com/llvm/llvm-project/commit/d08482924efe8b2c44913583af7b8f60a29975d1.diff

LOG: [clang-tidy] fix false positive in cppcoreguidelines-missing-std-forward (#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](https://github.com/llvm/llvm-project/issues/68105)

Co-authored-by: huqizhi <836744285 at qq.com>

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
index 0b85ea19735eef..370de12999aceb 100644
--- 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
index 08ade306b5a077..1bd5a72126c10b 100644
--- 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
index b9720db272e406..443f338ba2046a 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,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