[PATCH] D80603: add isAtPosition narrowing matcher for parmVarDecl

[Vy Nguyen via Phabricator via cfe-commits]

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