[PATCH] D73028: Extract ExprTraversal class from Expr

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D73028#1839572 <https://reviews.llvm.org/D73028#1839572>, @rsmith wrote:

> In D73028#1839494 <https://reviews.llvm.org/D73028#1839494>, @steveire wrote:
>
> > In D73028#1839383 <https://reviews.llvm.org/D73028#1839383>, @rsmith wrote:
> >
> > >
> >
> >
> > The follow-up is here: https://reviews.llvm.org/D73029 .
> >
> > I have the need to change the ascending traversal implemented in ASTContext.cpp with a map (populated while descending) to make it skip nodes too during parent traversal.
> >
> > I didn't want to have descending traversal in Expr.cpp and ascending traversal in ASTContext.cpp as they would be likely to go out of sync, so moving this implementation here allows extending the purpose in the follow-up.
>
>
> The ascending traversal should not be in the AST library at all. If tooling or static analysis wants this, that's fine, but navigating upwards is not something the AST supports, and we do not want any part of clang proper developing a reliance on having a parent map, so we're working on moving the parent map out of `ASTContext` (with a plan to eventually move it out of the AST library). See D71313 <https://reviews.llvm.org/D71313> for related work in this area.


Thank you for pointing this out, I was unaware of that direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73028