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

Wed Oct 7 05:33:53 PDT 2020

aaron.ballman added a comment.

In D88275#2313342 <https://reviews.llvm.org/D88275#2313342>, @ymandel wrote:

> 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?

Thank you for your patience while we sort this out. I've pinged @steveire off-list, so hopefully he'll respond with his thoughts. If we don't hear from Stephen in the next week or so, I think we should proceed with your proposed patch to get you unblocked (adding one more "ignore implicit" variant isn't the end of the world, I just want us to be thoughtful about whether we want to add new matchers in this space or to work on the traversal mode instead).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88275