steveire 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. Thanks for the heads-up - I'm trying to reach the conclusion from this information. Are you saying that 1. this patch shouldn't be committed, so down-traversal-skipping should remain in Expr.cpp 2. D73029 <https://reviews.llvm.org/D73029> should be changed to implement the up-traversal-skipping directly in ParentMapContext.cpp Is that the tight conclusion? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73028/new/ https://reviews.llvm.org/D73028 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
