Address review comments and add another test.

Inline a function instead of needing a good name for it, rewrite
explanatory comments, and add a test to demonstrate lack of behavior
change for common circumstances.

Author: bazuzi

clang/include/clang/Analysis/FlowSensitive/ASTOps.h

@@ -155,13 +155,16 @@ struct ReferencedDecls {
   /// the surrounding function) that the lambda captures. Captured local
   /// variables are already included in `Locals` above.
   ///
-  /// This set also includes any referenced parameters of functions other than
-  /// the function whose body is targeted for visitation. When calling
-  /// `getReferencedDecls(const Stmt& S)`, there is no such targeted function,
-  /// so any referenced function parameters are included in this set. This
-  /// supports the collection of ReferencedDecls from a DeclStmt constructed for
-  /// lambda init-capture VarDecls for the purpose of performing a dataflow
-  /// analysis on the declaration/initialization.
+  /// When analyzing a standalone `Stmt` directly, this set includes any
+  /// referenced function parameters. This supports the collection of
+  /// ReferencedDecls from a DeclStmt constructed for lambda init-capture
+  /// VarDecls for the purpose of performing a dataflow analysis on the
+  /// declaration/initialization. When analyzing any `FunctionDecl`, this set
+  /// would also include any parameters of other functions that are referenced
+  /// in the analyzed function's body, though there is no known case where this
+  /// happens. If other (especially non-lambda-related) cases are found, this
+  /// group of parameters could be moved out of `LambdaCapturedParams` into a
+  /// separate set.
   llvm::SetVector<const ParmVarDecl *> LambdaCapturedParams;
 };
 

clang/lib/Analysis/FlowSensitive/ASTOps.cpp

@@ -183,17 +183,6 @@ static void insertIfFunction(const Decl &D,
     Funcs.insert(FD);
 }
 
-static void insertIfParamOfNotThePrimaryFunction(
-    const Decl &D, llvm::SetVector<const ParmVarDecl *> &Locals,
-    const FunctionDecl *PrimaryFunction) {
-  if (auto *PVD = dyn_cast<ParmVarDecl>(&D)) {
-    if (!PrimaryFunction ||
-        PVD->getParentFunctionOrMethod() != PrimaryFunction) {
-      Locals.insert(PVD);
-    }
-  }
-}
-
 static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
   // Use getCalleeDecl instead of getMethodDecl in order to handle
   // pointer-to-member calls.
@@ -212,8 +201,8 @@ static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
 class ReferencedDeclsVisitor : public AnalysisASTVisitor {
 public:
   ReferencedDeclsVisitor(ReferencedDecls &Referenced,
-                         const FunctionDecl *PrimaryFunction)
-      : Referenced(Referenced), PrimaryFunction(PrimaryFunction) {}
+                         const FunctionDecl *AnalyzedFunction)
+      : Referenced(Referenced), AnalyzedFunction(AnalyzedFunction) {}
 
   void traverseConstructorInits(const CXXConstructorDecl *Ctor) {
     for (const CXXCtorInitializer *Init : Ctor->inits()) {
@@ -247,8 +236,17 @@ class ReferencedDeclsVisitor : public AnalysisASTVisitor {
     insertIfGlobal(*E->getDecl(), Referenced.Globals);
     insertIfLocal(*E->getDecl(), Referenced.Locals);
     insertIfFunction(*E->getDecl(), Referenced.Functions);
-    insertIfParamOfNotThePrimaryFunction(
-        *E->getDecl(), Referenced.LambdaCapturedParams, PrimaryFunction);
+
+    // Collect referenced parameters of functions other than the function being
+    // analyzed, or of any function if we are analyzing a standalone statement.
+    // See comments on `LambdaCapturedParams` for motivations.
+    if (auto *P = dyn_cast<ParmVarDecl>(E->getDecl())) {
+      if (!AnalyzedFunction ||
+          P->getParentFunctionOrMethod() != AnalyzedFunction) {
+        Referenced.LambdaCapturedParams.insert(P);
+      }
+    }
+
     return true;
   }
 
@@ -286,7 +284,7 @@ class ReferencedDeclsVisitor : public AnalysisASTVisitor {
 private:
   ReferencedDecls &Referenced;
   // May be null, if we are visiting a statement that is not a function body.
-  const FunctionDecl *const PrimaryFunction;
+  const FunctionDecl *const AnalyzedFunction;
 };
 
 ReferencedDecls getReferencedDecls(const FunctionDecl &FD) {

clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp

@@ -180,4 +180,25 @@ TEST(ASTOpsTest, LambdaInitCapture) {
               UnorderedElementsAre(IDecl));
 }
 
+TEST(ASTOpsTest, NoLambdaCapturedParamsWhenAnalyzingFunctionNotLambdaOperator) {
+  std::string Code = R"cc(
+    void func(int I) {
+      I++; // Parameters of the function being analyzed don't get included in `LambdaCapturedParams`.
+      auto Lambda = [&I](int L) { // We don't see the capture of `I` when analyzing `func`.
+        L++; // We don't see the lambda body when analyzing `func`.
+        I++; // This is a parameter of the function being analyzed, and it's not seen when analyzing `func`.
+      };
+    }
+  )cc";
+  std::unique_ptr<ASTUnit> Unit =
+      tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"});
+  auto &ASTCtx = Unit->getASTContext();
+
+  ASSERT_EQ(ASTCtx.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+  auto *Func = cast<FunctionDecl>(findValueDecl(ASTCtx, "func"));
+  ASSERT_NE(Func, nullptr);
+  EXPECT_THAT(getReferencedDecls(*Func).LambdaCapturedParams, IsEmpty());
+}
+
 } // namespace