steveire added a comment. In D73028#1839644 <https://reviews.llvm.org/D73028#1839644>, @steveire wrote:
> 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 right conclusion? Implemented that in https://reviews.llvm.org/D73388 I think this and https://reviews.llvm.org/D73029 can be closed in favor of that one. 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
