https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/170311
>From a755b296cd7807cb96c864c0d7f45b27d7ddfa6a Mon Sep 17 00:00:00 2001 From: Samira Bakon <[email protected]> Date: Tue, 2 Dec 2025 10:17:06 -0500 Subject: [PATCH 1/2] [clang][dataflow]Add parameters of other functions to `LambdaCapturedParams`. "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. --- .../clang/Analysis/FlowSensitive/ASTOps.h | 8 ++++ clang/lib/Analysis/FlowSensitive/ASTOps.cpp | 24 ++++++++++-- .../Analysis/FlowSensitive/ASTOpsTest.cpp | 38 +++++++++++++++++-- 3 files changed, 63 insertions(+), 7 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h index 3e57f30dd2053..a6c390a0bbb42 100644 --- a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h +++ b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h @@ -154,6 +154,14 @@ 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. + /// + /// 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. llvm::SetVector<const ParmVarDecl *> LambdaCapturedParams; }; diff --git a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp index e8113fc094037..55a2fad72a3dd 100644 --- a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp +++ b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp @@ -183,6 +183,17 @@ 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. @@ -200,8 +211,9 @@ static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) { class ReferencedDeclsVisitor : public AnalysisASTVisitor { public: - ReferencedDeclsVisitor(ReferencedDecls &Referenced) - : Referenced(Referenced) {} + ReferencedDeclsVisitor(ReferencedDecls &Referenced, + const FunctionDecl *PrimaryFunction) + : Referenced(Referenced), PrimaryFunction(PrimaryFunction) {} void traverseConstructorInits(const CXXConstructorDecl *Ctor) { for (const CXXCtorInitializer *Init : Ctor->inits()) { @@ -235,6 +247,8 @@ 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); return true; } @@ -271,11 +285,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 PrimaryFunction; }; 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 +323,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..9bf35e8bce244 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,32 @@ 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)); +} + } // namespace >From 5323cdb3c58cb123061331823efefcb2da90fa5a Mon Sep 17 00:00:00 2001 From: Samira Bakon <[email protected]> Date: Tue, 2 Dec 2025 15:31:29 -0500 Subject: [PATCH 2/2] 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. --- .../clang/Analysis/FlowSensitive/ASTOps.h | 17 ++++++----- clang/lib/Analysis/FlowSensitive/ASTOps.cpp | 30 +++++++++---------- .../Analysis/FlowSensitive/ASTOpsTest.cpp | 21 +++++++++++++ 3 files changed, 45 insertions(+), 23 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h index a6c390a0bbb42..6804f3ba4235f 100644 --- a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h +++ b/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; }; diff --git a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp index 55a2fad72a3dd..75a946c3ffccc 100644 --- a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp +++ b/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) { diff --git a/clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp b/clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp index 9bf35e8bce244..866c8edd80cb6 100644 --- a/clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp +++ b/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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
