[clang][dataflow] Add parameters of other functions to `LambdaCaptured…

[bazuzi]

…Params`.

"Other functions" here are functions other than the FunctionDecl provided to getReferencedDecls. Adding the parameters to LambdaCapturedParams because the only known triggering circumstance is a synthetic Stmt used for dataflow analysis of lambda init-captures where the initializer references the parameter(s) of the function surrounding the LambdaExpr.

[ymand]

This wording feels general (and hence is a bit hard to parse). Could you be more specific, like

"of the surrounding function that introduces the lambda whose body is targeted for visitation"

? Or, is there actually more general functionality here?

[bazuzi]

This change doesn't affect the visitation of lambda bodies.

The code change is quite general here, as you can see from the limited use of conditionals. The expected impact is quite narrow, though, because the triggering circumstances are rare and very specific. I rewrote this block of comments to try and clarify. Does that help?

[llvmbot]

@llvm/pr-subscribers-clang-analysis

@llvm/pr-subscribers-clang

Author: Samira Bakon (bazuzi)

Changes

…Params`.

"Other functions" here are functions other than the FunctionDecl provided to getReferencedDecls. Adding the parameters to LambdaCapturedParams because the only known triggering circumstance is a synthetic Stmt used for dataflow analysis of lambda init-captures where the initializer references the parameter(s) of the function surrounding the LambdaExpr.

---

Full diff: https://github.com/llvm/llvm-project/pull/170311.diff

3 Files Affected:

• (modified) clang/include/clang/Analysis/FlowSensitive/ASTOps.h (+11)
• (modified) clang/lib/Analysis/FlowSensitive/ASTOps.cpp (+18-4)
• (modified) clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp (+56-3)

diff --git a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h
index 3e57f30dd2053..6804f3ba4235f 100644
--- a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h
+++ b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h
@@ -154,6 +154,17 @@ struct ReferencedDecls {
   /// When analyzing a lambda's call operator, the set of all parameters (from
   /// the surrounding function) that the lambda captures. Captured local
   /// variables are already included in `Locals` above.
+  ///
+  /// 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;
 };
 
diff --git a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp
index e8113fc094037..75a946c3ffccc 100644
--- a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp
+++ b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp
@@ -200,8 +200,9 @@ static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
 
 class ReferencedDeclsVisitor : public AnalysisASTVisitor {
 public:
-  ReferencedDeclsVisitor(ReferencedDecls &Referenced)
-      : Referenced(Referenced) {}
+  ReferencedDeclsVisitor(ReferencedDecls &Referenced,
+                         const FunctionDecl *AnalyzedFunction)
+      : Referenced(Referenced), AnalyzedFunction(AnalyzedFunction) {}
 
   void traverseConstructorInits(const CXXConstructorDecl *Ctor) {
     for (const CXXCtorInitializer *Init : Ctor->inits()) {
@@ -235,6 +236,17 @@ class ReferencedDeclsVisitor : public AnalysisASTVisitor {
     insertIfGlobal(*E->getDecl(), Referenced.Globals);
     insertIfLocal(*E->getDecl(), Referenced.Locals);
     insertIfFunction(*E->getDecl(), Referenced.Functions);
+
+    // 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;
   }
 
@@ -271,11 +283,13 @@ 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 AnalyzedFunction;
 };
 
 ReferencedDecls getReferencedDecls(const FunctionDecl &FD) {
   ReferencedDecls Result;
-  ReferencedDeclsVisitor Visitor(Result);
+  ReferencedDeclsVisitor Visitor(Result, &FD);
   Visitor.TraverseStmt(FD.getBody());
   if (const auto *CtorDecl = dyn_cast<CXXConstructorDecl>(&FD))
     Visitor.traverseConstructorInits(CtorDecl);
@@ -307,7 +321,7 @@ ReferencedDecls getReferencedDecls(const FunctionDecl &FD) {
 
 ReferencedDecls getReferencedDecls(const Stmt &S) {
   ReferencedDecls Result;
-  ReferencedDeclsVisitor Visitor(Result);
+  ReferencedDeclsVisitor Visitor(Result, nullptr);
   Visitor.TraverseStmt(const_cast<Stmt *>(&S));
   return Result;
 }
diff --git a/clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp b/clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp
index e086ea3c892f1..866c8edd80cb6 100644
--- a/clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp
@@ -10,6 +10,8 @@
 #include "TestingSupport.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclGroup.h"
+#include "clang/AST/Stmt.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Frontend/ASTUnit.h"
@@ -29,8 +31,11 @@ using ast_matchers::cxxRecordDecl;
 using ast_matchers::hasName;
 using ast_matchers::hasType;
 using ast_matchers::initListExpr;
+using ast_matchers::isInitCapture;
 using ast_matchers::match;
+using ast_matchers::parmVarDecl;
 using ast_matchers::selectFirst;
+using ast_matchers::varDecl;
 using test::findValueDecl;
 using testing::IsEmpty;
 using testing::UnorderedElementsAre;
@@ -118,9 +123,8 @@ TEST(ASTOpsTest, ReferencedDeclsLocalsNotParamsOrStatics) {
 TEST(ASTOpsTest, LambdaCaptures) {
   std::string Code = R"cc(
     void func(int CapturedByRef, int CapturedByValue, int NotCaptured) {
-    int Local;
-      auto Lambda = [&CapturedByRef, CapturedByValue, &Local](int LambdaParam) {
-      };
+      int Local;
+      auto Lambda = [&CapturedByRef, CapturedByValue, &Local](int LambdaParam) {};
     }
   )cc";
   std::unique_ptr<ASTUnit> Unit =
@@ -148,4 +152,53 @@ TEST(ASTOpsTest, LambdaCaptures) {
   EXPECT_THAT(ForLambda.Locals, IsEmpty());
 }
 
+TEST(ASTOpsTest, LambdaInitCapture) {
+  std::string Code = R"cc(
+    void func(int I) {
+      auto Lambda = [C = I]() {};
+    }
+  )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 *IDecl = selectFirst<ParmVarDecl>(
+      "i", match(parmVarDecl(hasName("I")).bind("i"), ASTCtx));
+
+  // Synthesize a temporary DeclStmt for the assignment of Q to
+  // its initializing expression. This is an unusual pattern that does not
+  // perfectly reflect the CFG or AST for declaration or assignment of an
+  // init-capture, but is used for dataflow analysis, which requires a Stmt and
+  // not just a VarDecl with an initializer.
+  auto *CDecl = selectFirst<VarDecl>(
+      "c", match(varDecl(isInitCapture()).bind("c"), ASTCtx));
+  DeclStmt DS(DeclGroupRef(const_cast<VarDecl *>(CDecl)), CDecl->getBeginLoc(),
+              CDecl->getEndLoc());
+  EXPECT_THAT(getReferencedDecls(DS).LambdaCapturedParams,
+              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

[ymand]

Thanks!

[ymand]

Maybe a little simpler (of possibly losing some information):

... This supports the collection of
/// ReferencedDecls from a DeclStmt constructed for analysis of lambda init-capture
/// VarDecls.

[ymand]

super nit: I might preface with "Note:" to highlight that this is an aside and no longer describing the variable.