Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][ASTVisitor] Visit HoldingVar from BindingDecl. #117858

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

legrosbuffle
Copy link
Contributor

Tuple-like types introduce VarDecls in the AST for their "holding vars", but AST visitors do not visit those. As a result the VarDecl for the holding var is orphaned when trying to retreive its parents.

Fix a FlowSensitive test that assumes that only a BindingDecl is introduced with the given name (the matcher now can also reach the VarDecl for the holding var).

Tuple-like types introduce `VarDecl`s in the AST for their "holding
vars", but AST visitors do not visit those. As a result the `VarDecl`
for the holding var is orphaned when trying to retreive its parents.

Fix a `FlowSensitive` test that assumes that only a `BindingDecl`
is introduced with the given name (the matcher now can also reach
the `VarDecl` for the holding var).
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html labels Nov 27, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 27, 2024

@llvm/pr-subscribers-clang

Author: Clement Courbet (legrosbuffle)

Changes

Tuple-like types introduce VarDecls in the AST for their "holding vars", but AST visitors do not visit those. As a result the VarDecl for the holding var is orphaned when trying to retreive its parents.

Fix a FlowSensitive test that assumes that only a BindingDecl is introduced with the given name (the matcher now can also reach the VarDecl for the holding var).


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

3 Files Affected:

  • (modified) clang/include/clang/AST/RecursiveASTVisitor.h (+4-1)
  • (modified) clang/unittests/AST/ASTContextParentMapTest.cpp (+49)
  • (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (+13-4)
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index 2b35997bd539ac..3ff60555c7873c 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2143,8 +2143,11 @@ DEF_TRAVERSE_DECL(DecompositionDecl, {
 })
 
 DEF_TRAVERSE_DECL(BindingDecl, {
-  if (getDerived().shouldVisitImplicitCode())
+  if (getDerived().shouldVisitImplicitCode()) {
     TRY_TO(TraverseStmt(D->getBinding()));
+    if (const auto HoldingVar = D->getHoldingVar())
+      TRY_TO(TraverseDecl(HoldingVar));
+  }
 })
 
 DEF_TRAVERSE_DECL(MSPropertyDecl, { TRY_TO(TraverseDeclaratorHelper(D)); })
diff --git a/clang/unittests/AST/ASTContextParentMapTest.cpp b/clang/unittests/AST/ASTContextParentMapTest.cpp
index 515dfb99e1126d..9af0a46817a25f 100644
--- a/clang/unittests/AST/ASTContextParentMapTest.cpp
+++ b/clang/unittests/AST/ASTContextParentMapTest.cpp
@@ -148,5 +148,54 @@ TEST(GetParents, FriendTypeLoc) {
               ElementsAre(DynTypedNode::create(FrA)));
 }
 
+TEST(GetParents, UserDefinedTupleLikeTypes) {
+  MatchVerifier<VarDecl> Verifier;
+  EXPECT_TRUE(Verifier.match(
+      R"(
+namespace std {
+
+using size_t = __typeof(sizeof(int));
+
+template <typename T>
+struct tuple_size;
+
+template <typename T>
+struct tuple_size<T&> : tuple_size<T>{};
+
+template <typename T>
+requires requires { tuple_size<T>::value; }
+struct tuple_size<const T> : tuple_size<T>{};
+
+
+template<size_t i, typename T>
+struct tuple_element;
+
+
+}  // namespace std
+
+struct Decomposable {};
+
+template<> struct std::tuple_size<Decomposable> {
+  static constexpr size_t value = 2;
+};
+
+template<std::size_t i> struct std::tuple_element<i, Decomposable> {
+  using type = int;
+};
+
+template<std::size_t i> struct std::tuple_element<i, const Decomposable> {
+  using type = const int;
+};
+
+template<std::size_t i>
+const int& get(const Decomposable& d);
+
+void F(const Decomposable& d) {
+    const auto& [x, y] = d;
+}
+)",
+      varDecl(hasName("x"), hasAncestor(decompositionDecl())), Lang_CXX20));
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 39e7001393e5e9..e10e4611ab8c60 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -143,6 +143,15 @@ const Formula &getFormula(const ValueDecl &D, const Environment &Env) {
   return cast<BoolValue>(Env.getValue(D))->formula();
 }
 
+const BindingDecl *findBindingDecl(const char *Name, ASTContext &ASTCtx) {
+  using ast_matchers::bindingDecl;
+  using ast_matchers::hasName;
+  auto TargetNodes =
+      ast_matchers::match(bindingDecl(hasName(Name)).bind("v"), ASTCtx);
+  assert(TargetNodes.size() == 1 && "Name must be unique");
+  return ast_matchers::selectFirst<BindingDecl>("v", TargetNodes);
+}
+
 TEST(TransferTest, CNotSupported) {
   TestInputs Inputs("void target() {}");
   Inputs.Language = TestLanguage::Lang_C89;
@@ -5515,10 +5524,10 @@ TEST(TransferTest, StructuredBindingAssignFromTupleLikeType) {
         ASSERT_THAT(Results.keys(), UnorderedElementsAre("p1", "p2"));
         const Environment &Env1 = getEnvironmentAtAnnotation(Results, "p1");
 
-        const ValueDecl *BoundFooDecl = findValueDecl(ASTCtx, "BoundFoo");
+        const ValueDecl *BoundFooDecl = findBindingDecl("BoundFoo", ASTCtx);
         ASSERT_THAT(BoundFooDecl, NotNull());
 
-        const ValueDecl *BoundBarDecl = findValueDecl(ASTCtx, "BoundBar");
+        const ValueDecl *BoundBarDecl = findBindingDecl("BoundBar", ASTCtx);
         ASSERT_THAT(BoundBarDecl, NotNull());
 
         const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
@@ -5596,10 +5605,10 @@ TEST(TransferTest, StructuredBindingAssignRefFromTupleLikeType) {
         ASSERT_THAT(Results.keys(), UnorderedElementsAre("p1", "p2"));
         const Environment &Env1 = getEnvironmentAtAnnotation(Results, "p1");
 
-        const ValueDecl *BoundFooDecl = findValueDecl(ASTCtx, "BoundFoo");
+        const ValueDecl *BoundFooDecl = findBindingDecl("BoundFoo", ASTCtx);
         ASSERT_THAT(BoundFooDecl, NotNull());
 
-        const ValueDecl *BoundBarDecl = findValueDecl(ASTCtx, "BoundBar");
+        const ValueDecl *BoundBarDecl = findBindingDecl("BoundBar", ASTCtx);
         ASSERT_THAT(BoundBarDecl, NotNull());
 
         const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants