[PATCH] D88275: [ASTMatchers] Add matcher `hasParentIgnoringImplicit`.

Yitzhak Mandelbaum via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 5 19:29:16 PDT 2020


ymandel added a comment.

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

> In D88275#2303283 <https://reviews.llvm.org/D88275#2303283>, @ymandel wrote:
>
>>> I'm not concerned about the basic idea behind the proposed matcher, I'm only worried we're making AST matching more confusing by having two different ways of inconsistently accomplishing the same-ish thing.
>>
>> Aaron, I appreciate this concern, but I would argue that this matcher isn't making things any worse. We already have the various `ignoringImplicit` matchers, and this new one simply parallels those, but for parents. So, it is in some sense "completing" an existing API, which together is an alternative to  `traverse`.
>
> I'm not certain I agree with that reasoning because you can extend it to literally any match that may interact with implicit nodes, which is the whole point to the spelled in source traversal mode. I'm not certain it's a good design for us to continue to add one-off ways to ignore implicit nodes.

I appreciate your point, but I'm personally inclined to allow progress while we figure these larger issues out.  That said, I'm in no rush and would like a solution that we're both happy with. How do you propose we proceed, especially given that `traverse` does not currently support this case? It seems that progress is in part blocked on hearing back from steveire, but it's been over a week since you added him to the review thread. Is there some other way to ping him?

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88275



More information about the cfe-commits mailing list