oontvoo added a comment.

In D80603#2057014 <https://reviews.llvm.org/D80603#2057014>, @aaron.ballman 
wrote:

> Does the `hasParameter()` matcher not already do this?


Yes, it does. But we'd like it to be able to match on the parmVarDecl().



================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7018
+AST_MATCHER_P(clang::ParmVarDecl, isAtPosition, unsigned, N) {
+  return parmVarDecl(hasAncestor(functionDecl(
+                         hasParameter(N, parmVarDecl().bind("this_decl")))),
----------------
ymandel wrote:
> I think that `hasAncestor` will keep going up the chain of ancestors if it 
> fails the match on the first surrounding `functionDecl`. That could make this 
> matcher very expensive.  Why hasParent (or, whatever combination of 
> `hasParent` and other matchers needed to precisely describe the immediately 
> surrounding `functionDecl`)?
> Why [not] hasParent ?

The direct parent of the param is a TypeLoc, so I thought `hasAncestor()` was 
more convenient. But I've changed it to two nested `hasParent()` now.



================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7022
+      .matches(Node, Finder, Builder);
+}
+
----------------
gribozavr2 wrote:
> Could you move it closer to other parameter-related matchers, maybe near the 
> definition of `forEachArgumentWithParam`?
Moved it to after hasAncestor.
(Can't move to near forEachArgumentWithParam because that's before hasAncestor 
and hasParent, which are needed by this)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80603/new/

https://reviews.llvm.org/D80603



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to