gribozavr2 added a comment.

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

> I'm sorry if I'm being dense, but `hasParameter`  traverses to the 
> `ParmVarDecl`, so I'm not certain I understand why this new matcher is needed 
> as a public matcher. It seems like you can already accomplish this without 
> adding a second API to do effectively the same thing: 
> `functionDecl(hasParameter(0, parmVarDecl().bind("param")))`, can't you?


It is syntax sugar, true. Note that the proposed matcher is a narrowing matcher 
for `parmVarDecl()`, while your suggestion is a narrowing matcher for 
`functionDecl()`, so it is not an entirely apples-to-apples comparison. Think 
about use cases like: `declRefExpr(to(parmVarDecl(at(...))))`. To rewrite that 
with `hasParameter()`, we have to use `hasAncestor` to find the `functionDecl` 
first, and then compare the AST node pointers. So while it is possible to 
express this proposed operation today, it requires a complex matcher for such a 
conceptually simple operation.



================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4621
+template <typename DeclT>
+static bool __isParamAt(const clang::DeclContext *Context,
+                        const clang::ParmVarDecl &Node, unsigned N) {
----------------
Identifiers with double underscores are reserved. Also, this file typically 
does not define helpers.

My best suggestion is to define a local lambda in the AST_MATCHER_P, or just 
copy the code three times -- there is not that much duplication.


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