[ASTMatchers] Ignore parts of BindingDecls which are not spelled in source

[aaron.ballman]

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

2607

Can you help me to better understand the proposed change? I find this to be surprising behavior -- the thing the user wrote in the source are the binding declarations, the hidden thing they didn't write in the source was the decomposition declaration. e.g., the user introduced some names f, s, and t in the declaration so I would imagine that these *are* spelled in source. What I don't think is necessarily spelled in source is the decomposition declaration for the int[3] to bind to. So I was expecting this to be EXPECT_TRUE instead of EXPECT_FALSE.

[steveire]

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

2607

Can you help me to better understand the proposed change? I find this to be surprising behavior -- the thing the user wrote in the source are the binding declarations, the hidden thing they didn't write in the source was the decomposition declaration. e.g., the user introduced some names f, s, and t in the declaration so I would imagine that these *are* spelled in source. What I don't think is necessarily spelled in source is the decomposition declaration for the int[3] to bind to.

Indeed. The bindingDecl matches, but the has(expr()) part does not, after this change: https://godbolt.org/z/Mqb9Mx

[steveire]

Add test

[aaron.ballman]

LGTM!

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

2607

The test changes nicely clarify what's going on, thank you.

clang/include/clang/AST/ASTNodeTraverser.h

public:
 
   void VisitDecompositionDecl(const DecompositionDecl *D) {
     VisitVarDecl(D);
     for (const auto *B : D->bindings())
       Visit(B);
   }
 
   void VisitBindingDecl(const BindingDecl *D) {
+    if (Traversal == TK_IgnoreUnlessSpelledInSource)
+      return;
     if (const auto *E = D->getBinding())
       Visit(E);
   }
 
   void VisitFileScopeAsmDecl(const FileScopeAsmDecl *D) {
     Visit(D->getAsmString());
   }
 

clang/lib/ASTMatchers/ASTMatchFinder.cpp

if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(DeclNode)) {
     if (SK == TSK_ExplicitInstantiationDeclaration ||
         SK == TSK_ExplicitInstantiationDefinition)
       ScopedChildren = true;
   } else if (const auto *FD = dyn_cast<FunctionDecl>(DeclNode)) {
     if (FD->isDefaulted())
       ScopedChildren = true;
     if (FD->isTemplateInstantiation())
       ScopedTraversal = true;
+  } else if (isa<BindingDecl>(DeclNode)) {
+    ScopedChildren = true;
   }
 
   ASTNodeNotSpelledInSourceScope RAII1(this, ScopedTraversal);
   ASTChildrenNotSpelledInSourceScope RAII2(this, ScopedChildren);
 
   match(*DeclNode);
   return RecursiveASTVisitor<MatchASTVisitor>::TraverseDecl(DeclNode);
 }

clang/unittests/AST/ASTTraverserTest.cpp

 
 void hasDefaultArg(int i, int j = 0)
 {
 }
 void callDefaultArg()
 {
   hasDefaultArg(42);
 }
+
+void decomposition()
+{
+    int arr[3];
+    auto &[f, s, t] = arr;
+
+    f = 42;
+}
+
 )cpp",
                                        {"-std=c++20"});
 
   {
     auto BN = ast_matchers::match(
         cxxRecordDecl(hasName("Record"), unless(isImplicit())).bind("rec"),
         AST2->getASTContext());
     EXPECT_EQ(BN.size(), 1u);
)cpp");
     EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource,
                             BN[0].getNodeAs<CallExpr>("funcCall")),
               R"cpp(
 CallExpr
 |-DeclRefExpr 'hasDefaultArg'
 `-IntegerLiteral
 )cpp");
   }
+
+  {
+    auto FN = ast_matchers::match(
+        functionDecl(hasName("decomposition"),
+                     hasDescendant(decompositionDecl().bind("decomp"))),
+        AST2->getASTContext());
+    EXPECT_EQ(FN.size(), 1u);
+
+    EXPECT_EQ(
+        dumpASTString(TK_AsIs, FN[0].getNodeAs<DecompositionDecl>("decomp")),
+        R"cpp(
+DecompositionDecl ''
+|-DeclRefExpr 'arr'
+|-BindingDecl 'f'
+| `-ArraySubscriptExpr
+|   |-ImplicitCastExpr
+|   | `-DeclRefExpr ''
+|   `-IntegerLiteral
+|-BindingDecl 's'
+| `-ArraySubscriptExpr
+|   |-ImplicitCastExpr
+|   | `-DeclRefExpr ''
+|   `-IntegerLiteral
+`-BindingDecl 't'
+  `-ArraySubscriptExpr
+    |-ImplicitCastExpr
+    | `-DeclRefExpr ''
+    `-IntegerLiteral
+)cpp");
+
+    EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource,
+                            FN[0].getNodeAs<DecompositionDecl>("decomp")),
+              R"cpp(
+DecompositionDecl ''
+|-DeclRefExpr 'arr'
+|-BindingDecl 'f'
+|-BindingDecl 's'
+`-BindingDecl 't'
+)cpp");
+  }
 }
 
 TEST(Traverse, IgnoreUnlessSpelledInSourceTemplateInstantiations) {
 
   auto AST = buildASTFromCode(R"cpp(
 template<typename T>
 struct TemplStruct {
   TemplStruct() {}

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

)cpp";
     auto M = expr().bind("allExprs");
     EXPECT_TRUE(matchAndVerifyResultTrue(
         Code, traverse(TK_AsIs, M),
         std::make_unique<VerifyIdIsBoundTo<Expr>>("allExprs", 7)));
     EXPECT_TRUE(matchAndVerifyResultTrue(
         Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
         std::make_unique<VerifyIdIsBoundTo<Expr>>("allExprs", 1)));
   }
+
+  Code = R"cpp(
+void foo()
+{
+    int arr[3];
+    auto &[f, s, t] = arr;
+
+    f = 42;
+}
+  )cpp";
+  {
+    auto M = bindingDecl(hasName("f"));
+    EXPECT_TRUE(
+        matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++17"}));
+    EXPECT_TRUE(
+        matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+                             true, {"-std=c++17"}));
+  }
+  {
+    auto M = bindingDecl(hasName("f"), has(expr()));
+    EXPECT_TRUE(
+        matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++17"}));
+    EXPECT_FALSE(
+        matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+                             true, {"-std=c++17"}));
+  }
+  {
+    auto M = integerLiteral(hasAncestor(bindingDecl(hasName("f"))));
+    EXPECT_TRUE(
+        matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++17"}));
+    EXPECT_FALSE(
+        matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+                             true, {"-std=c++17"}));
+  }
+  {
+    auto M = declRefExpr(hasAncestor(bindingDecl(hasName("f"))));
+    EXPECT_TRUE(
+        matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++17"}));
+    EXPECT_FALSE(
+        matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+                             true, {"-std=c++17"}));
+  }
 }
 
 TEST(Traversal, traverseNoImplicit) {
   StringRef Code = R"cpp(
 struct NonTrivial {
     NonTrivial() {}
     NonTrivial(const NonTrivial&) {}
     NonTrivial& operator=(const NonTrivial&) { return *this; }