[PATCH] D73028: Extract ExprTraversal class from Expr

2020-01-24 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment. In D73028#1839644 , @steveire wrote: > In D73028#1839572 , @rsmith wrote: > > > In D73028#1839494 , @steveire > > wrote: > > > > > In

[PATCH] D73028: Extract ExprTraversal class from Expr

2020-01-24 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment. In D73028#1839572 , @rsmith wrote: > In D73028#1839494 , @steveire wrote: > > > In D73028#1839383 , @rsmith wrote: > > > > > > > > > > > The

[PATCH] D73028: Extract ExprTraversal class from Expr

2020-01-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D73028#1839572 , @rsmith wrote: > In D73028#1839494 , @steveire wrote: > > > In D73028#1839383 , @rsmith wrote: > > > > > > > > > > > The

[PATCH] D73028: Extract ExprTraversal class from Expr

2020-01-24 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D73028#1839494 , @steveire wrote: > In D73028#1839383 , @rsmith wrote: > > > > > > The follow-up is here: https://reviews.llvm.org/D73029 . > > I have the need to change the ascending

[PATCH] D73028: Extract ExprTraversal class from Expr

2020-01-24 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment. In D73028#1839383 , @rsmith wrote: > > An addition to the API will be concerned with ascending through the AST in > > different traversal modes. > > Ascending through the AST is not possibly by design. (For example, we share >

[PATCH] D73028: Extract ExprTraversal class from Expr

2020-01-24 Thread Stephen Kelly via Phabricator via cfe-commits
steveire updated this revision to Diff 240269. steveire marked an inline comment as done. steveire added a comment. Update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73028/new/ https://reviews.llvm.org/D73028 Files:

[PATCH] D73028: Extract ExprTraversal class from Expr

2020-01-24 Thread Stephen Kelly via Phabricator via cfe-commits
steveire updated this revision to Diff 240267. steveire added a comment. Update for review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73028/new/ https://reviews.llvm.org/D73028 Files: clang/include/clang/AST/Expr.h

[PATCH] D73028: Extract ExprTraversal class from Expr

2020-01-24 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. > An addition to the API will be concerned with ascending through the AST in > different traversal modes. Ascending through the AST is not possibly by design. (For example, we share AST nodes between template instantiations, and statement nodes don't contain parent

[PATCH] D73028: Extract ExprTraversal class from Expr

2020-01-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. This looks like it's an NFC patch where the only change is to hoist this functionality out into its own interface. Is that correct? If so, can you please be sure to add NFC to the commit log? Comment at:

[PATCH] D73028: Extract ExprTraversal class from Expr

2020-01-20 Thread Stephen Kelly via Phabricator via cfe-commits
steveire updated this revision to Diff 239072. steveire added a comment. Format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73028/new/ https://reviews.llvm.org/D73028 Files: clang/include/clang/AST/ExprTraversal.h

[PATCH] D73028: Extract ExprTraversal class from Expr

2020-01-20 Thread Stephen Kelly via Phabricator via cfe-commits
steveire created this revision. steveire added a reviewer: aaron.ballman. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. The current implementations are concerned with descending through the AST. An addition to the API will be concerned with ascending through the