[ASTMatchers] Fix parent-child traversal between functions and parms

[steveire]

Summary

Having to match on the typeLoc in AsIs mode is undesirable, but keep it
for now for compatbility.

[njames93]

I have no issues here, but see what the others say as well.
Also related post to cfe-dev - https://lists.llvm.org/pipermail/cfe-dev/2021-February/067629.html

clang/lib/ASTMatchers/ASTMatchFinder.cpp

148–151

nit: Inner If doesn't need braces, but I'd prefer the loop to have them.

[steveire]

Update

[klimek]

Can you explain the change and the before/after a bit more? Thanks!

[steveire]

In D96114#2548029, @klimek wrote:

Can you explain the change and the before/after a bit more? Thanks!

I'm not sure. Can you ask a more-specific question? Did you read the link that Nathan posted? Is the unit test unclear?

[steveire]

Clearing this out of my outgoing list.

clang/include/clang/AST/RecursiveASTVisitor.h

#define DEF_TRAVERSE_TMPL_INST(TMPLDECLKIND)                                   \
   bool TraverseTemplateInstantiations(TMPLDECLKIND##TemplateDecl *D);
   DEF_TRAVERSE_TMPL_INST(Class)
   DEF_TRAVERSE_TMPL_INST(Var)
   DEF_TRAVERSE_TMPL_INST(Function)
 #undef DEF_TRAVERSE_TMPL_INST
 
   bool dataTraverseNode(Stmt *S, DataRecursionQueue *Queue);
 
+  bool TraverseFunctionHelper(FunctionDecl *D);
+
 private:
   // These are helper methods used by more than one Traverse* method.
   bool TraverseTemplateParameterListHelper(TemplateParameterList *TPL);
 
   // Traverses template parameter lists of either a DeclaratorDecl or TagDecl.
   template <typename T>
   bool TraverseDeclTemplateParameterLists(T *D);
 
   bool TraverseTemplateArgumentLocsHelper(const TemplateArgumentLoc *TAL,
                                           unsigned Count);
   bool TraverseArrayTypeLocHelper(ArrayTypeLoc TL);
   bool TraverseRecordHelper(RecordDecl *D);
   bool TraverseCXXRecordHelper(CXXRecordDecl *D);
   bool TraverseDeclaratorHelper(DeclaratorDecl *D);
   bool TraverseDeclContextHelper(DeclContext *DC);
-  bool TraverseFunctionHelper(FunctionDecl *D);
   bool TraverseVarHelper(VarDecl *D);
   bool TraverseOMPExecutableDirective(OMPExecutableDirective *S);
   bool TraverseOMPLoopDirective(OMPLoopDirective *S);
   bool TraverseOMPClause(OMPClause *C);
 #define GEN_CLANG_CLAUSE_CLASS
 #define CLAUSE_CLASS(Enum, Str, Class) bool Visit##Class(Class *C);
 #include "llvm/Frontend/OpenMP/OMP.inc"
   /// Process clauses with list of variables.
bool RecursiveASTVisitor<Derived>::TraverseFunctionHelper(FunctionDecl *D) {
   }
   return true;
 }
 
 DEF_TRAVERSE_DECL(FunctionDecl, {
   // We skip decls_begin/decls_end, which are already covered by
   // TraverseFunctionHelper().
   ShouldVisitChildren = false;
-  ReturnValue = TraverseFunctionHelper(D);
+  ReturnValue = getDerived().TraverseFunctionHelper(D);
 })
 
 DEF_TRAVERSE_DECL(CXXDeductionGuideDecl, {
   // We skip decls_begin/decls_end, which are already covered by
   // TraverseFunctionHelper().
   ShouldVisitChildren = false;
-  ReturnValue = TraverseFunctionHelper(D);
+  ReturnValue = getDerived().TraverseFunctionHelper(D);
 })
 
 DEF_TRAVERSE_DECL(CXXMethodDecl, {
   // We skip decls_begin/decls_end, which are already covered by
   // TraverseFunctionHelper().
   ShouldVisitChildren = false;
-  ReturnValue = TraverseFunctionHelper(D);
+  ReturnValue = getDerived().TraverseFunctionHelper(D);
 })
 
 DEF_TRAVERSE_DECL(CXXConstructorDecl, {
   // We skip decls_begin/decls_end, which are already covered by
   // TraverseFunctionHelper().
   ShouldVisitChildren = false;
-  ReturnValue = TraverseFunctionHelper(D);
+  ReturnValue = getDerived().TraverseFunctionHelper(D);
 })
 
 // CXXConversionDecl is the declaration of a type conversion operator.
 // It's not a cast expression.
 DEF_TRAVERSE_DECL(CXXConversionDecl, {
   // We skip decls_begin/decls_end, which are already covered by
   // TraverseFunctionHelper().
   ShouldVisitChildren = false;
-  ReturnValue = TraverseFunctionHelper(D);
+  ReturnValue = getDerived().TraverseFunctionHelper(D);
 })
 
 DEF_TRAVERSE_DECL(CXXDestructorDecl, {
   // We skip decls_begin/decls_end, which are already covered by
   // TraverseFunctionHelper().
   ShouldVisitChildren = false;
-  ReturnValue = TraverseFunctionHelper(D);
+  ReturnValue = getDerived().TraverseFunctionHelper(D);
 })
 
 template <typename Derived>
 bool RecursiveASTVisitor<Derived>::TraverseVarHelper(VarDecl *D) {
   TRY_TO(TraverseDeclaratorHelper(D));
   // Default params are taken care of when we traverse the ParmVarDecl.
   if (!isa<ParmVarDecl>(D) &&
       (!D->isCXXForRangeDecl() || getDerived().shouldVisitImplicitCode()))

clang/lib/AST/ParentMapContext.cpp

 /// parents as defined by the \c RecursiveASTVisitor.
 ///
 /// Note that the relationship described here is purely in terms of AST
 /// traversal - there are other relationships (for example declaration context)
 /// in the AST that are better modeled by special matchers.
 class ParentMapContext::ParentMap::ASTVisitor
     : public RecursiveASTVisitor<ASTVisitor> {
 public:
-  ASTVisitor(ParentMap &Map) : Map(Map) {}
+  ASTVisitor(ParentMap &Map, ASTContext &Ctx) : Map(Map), Ctx(Ctx) {}
 
 private:
   friend class RecursiveASTVisitor<ASTVisitor>;
 
   using VisitorBase = RecursiveASTVisitor<ASTVisitor>;
 
   bool shouldVisitTemplateInstantiations() const { return true; }
 
private:
   }
 
   bool TraverseDecl(Decl *DeclNode) {
     return TraverseNode(
         DeclNode, DeclNode, [&] { return VisitorBase::TraverseDecl(DeclNode); },
         &Map.PointerParents);
   }
   bool TraverseTypeLoc(TypeLoc TypeLocNode) {
+    if (Ctx.getParentMapContext().getTraversalKind() ==
+        TK_IgnoreUnlessSpelledInSource) {
+      if (auto FPTL = TypeLocNode.getAs<FunctionProtoTypeLoc>()) {
+        for (unsigned I = 0, E = FPTL.getNumParams(); I != E; ++I) {
+          if (auto *P = FPTL.getParam(I)) {
+            if (!TraverseDecl(P))
+              return false;
+          }
+        }
+        return true;
+      }
+    }
+
     return TraverseNode(
         TypeLocNode, DynTypedNode::create(TypeLocNode),
         [&] { return VisitorBase::TraverseTypeLoc(TypeLocNode); },
         &Map.OtherParents);
   }
   bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNSLocNode) {
     return TraverseNode(
         NNSLocNode, DynTypedNode::create(NNSLocNode),
         [&] { return VisitorBase::TraverseNestedNameSpecifierLoc(NNSLocNode); },
         &Map.OtherParents);
   }
 
   // Using generic TraverseNode for Stmt would prevent data-recursion.
   bool dataTraverseStmtPre(Stmt *StmtNode) {
     addParent(StmtNode, &Map.PointerParents);
     ParentStack.push_back(DynTypedNode::create(*StmtNode));
     return true;
   }
   bool dataTraverseStmtPost(Stmt *StmtNode) {
     ParentStack.pop_back();
     return true;
   }
 
   ParentMap &Map;
+  ASTContext &Ctx;
   llvm::SmallVector<DynTypedNode, 16> ParentStack;
 };
 
 ParentMapContext::ParentMap::ParentMap(ASTContext &Ctx) {
-  ASTVisitor(*this).TraverseAST(Ctx);
+  ASTVisitor(*this, Ctx).TraverseAST(Ctx);
 }
 
 DynTypedNodeList ParentMapContext::getParents(const DynTypedNode &Node) {
   if (!Parents)
     // We build the parent map for the traversal scope (usually whole TU), as
     // hasAncestor can escape any subtree.
     Parents = std::make_unique<ParentMap>(ASTCtx);
   return Parents->getParents(getTraversalKind(), Node);
 }

clang/lib/ASTMatchers/ASTMatchFinder.cpp

bool findMatch(const DynTypedNode &DynNode) {
     // It's OK to always overwrite the bound nodes, as if there was
     // no match in this recursive branch, the result set is empty
     // anyway.
     *Builder = ResultBindings;
 
     return Matches;
   }
 
+  bool TraverseFunctionHelper(FunctionDecl *FD) {
+    if (Finder->isTraversalIgnoringImplicitNodes()) {
+      for (auto *P : FD->parameters())
+        if (!TraverseDecl(P)) {
+          return false;
+        }
+    }
+    return RecursiveASTVisitor<MatchChildASTVisitor>::TraverseFunctionHelper(
+        FD);
+  }
+
   // The following are overriding methods from the base visitor class.
   // They are public only to allow CRTP to work. They are *not *part
   // of the public API of this class.
   bool TraverseDecl(Decl *DeclNode) {
 
     if (DeclNode && DeclNode->isImplicit() &&
         Finder->isTraversalIgnoringImplicitNodes())
       return baseTraverse(*DeclNode);

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

EXPECT_TRUE(matches("class X { public: X(int); }; void x() { X x = X(0); }",
                       Constructor1Arg));
   EXPECT_TRUE(matches("class X { public: X(int); }; void x() { X x = 0; }",
                       Constructor1Arg));
   EXPECT_TRUE(
       notMatches("class X { public: X(int, int); }; void x() { X x(0, 0); }",
                  Constructor1Arg));
 }
 
+TEST(ASTMatchersTest, has_parmVarDecl) {
+
+  auto Code = R"cpp(
+void f(int ii)
+{
+
+}
+)cpp";
+
+  EXPECT_TRUE(matches(
+      Code,
+      traverse(TK_AsIs,
+               functionDecl(hasName("f"),
+                            has(typeLoc(has(parmVarDecl(hasName("ii"))))))),
+      langCxx20OrLater()));
+  EXPECT_TRUE(matches(
+      Code,
+      traverse(TK_IgnoreUnlessSpelledInSource,
+               functionDecl(hasName("f"), has(parmVarDecl(hasName("ii"))))),
+      langCxx20OrLater()));
+
+  EXPECT_TRUE(
+      matches(Code,
+              traverse(TK_AsIs, parmVarDecl(hasName("ii"),
+                                            hasParent(typeLoc(hasParent(
+                                                functionDecl(hasName("f"))))))),
+              langCxx20OrLater()));
+  EXPECT_TRUE(
+      matches(Code,
+              traverse(TK_IgnoreUnlessSpelledInSource,
+                       parmVarDecl(hasName("ii"),
+                                   hasParent(functionDecl(hasName("f"))))),
+              langCxx20OrLater()));
+}
+
 TEST(ASTMatchersTest, NamesMember_CXXDependentScopeMemberExpr) {
 
   // Member functions:
   {
     auto Code = "template <typename T> struct S{ void mem(); }; template "
                 "<typename T> void x() { S<T> s; s.mem(); }";
 
     EXPECT_TRUE(matches(

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

EXPECT_TRUE(matches(
       Code,
       traverse(TK_IgnoreUnlessSpelledInSource,
                varDecl(hasName("b"),
                        hasInitializer(declRefExpr(to(varDecl(hasName("c"))))),
                        hasParent(lambdaExpr(
                            forFunction(functionDecl(hasName("func13"))))))),
       langCxx20OrLater()));
 
+  EXPECT_TRUE(
+      matches(Code,
+              traverse(TK_IgnoreUnlessSpelledInSource,
+                       parmVarDecl(hasName("d"),
+                                   hasParent(lambdaExpr(forFunction(
+                                       functionDecl(hasName("func13"))))))),
+              langCxx20OrLater()));
+
   EXPECT_TRUE(matches(Code,
                       traverse(TK_IgnoreUnlessSpelledInSource,
                                compoundStmt(hasParent(lambdaExpr(forFunction(
                                    functionDecl(hasName("func13"))))))),
                       langCxx20OrLater()));
 
   EXPECT_TRUE(matches(
       Code,