steveire added a comment. In D61835#1505314 <https://reviews.llvm.org/D61835#1505314>, @aaron.ballman wrote:
> In D61835#1505280 <https://reviews.llvm.org/D61835#1505280>, @steveire wrote: > > > In D61835#1505228 <https://reviews.llvm.org/D61835#1505228>, @aaron.ballman > > wrote: > > > > > In D61835#1505202 <https://reviews.llvm.org/D61835#1505202>, @steveire > > > wrote: > > > > > > > 3. Anyone who wants traversal in the same way that dumping is done and > > > > who needs to call API on the instance which is provided by > > > > ASTNodeTraverser (which ASTDumper inherits) needs to use ASTDumper. For > > > > example my UI. See my EuroLLVM talk for more: > > > > https://steveire.wordpress.com/2019/04/30/the-future-of-ast-matching-refactoring-tools-eurollvm-and-accu/ > > > > > > > > > Do they? Why is the `ASTNodeTraverser` insufficient? > > > > > > It doesn't have the same behavior as `ASTDumper`, because `ASTDumper` > > "overrides" some `Visit` metthods. > > > I'm aware that they're different, but I may not have been sufficiently clear. > I'm saying that the only public APIs I think a user should be calling are > inherited from `ASTNodeTraverser` and not `ASTDumper`, so it is not required > to expose `ASTDumper` directly, only a way to get an `ASTNodeTraverser` > reference/pointer to an `ASTDumper` instance so that you get the correct > virtual dispatch. e.g., `ASTNodeTraverser > *getSomethingThatActsLikeAnASTDumper() { return new ASTDumper; }` Perhaps. One way to implement the 'less noise' version of AST output (ie removing pointer addresses etc http://ce.steveire.com/z/HaCLeO ) is to make it an API on the `TextNodeDumper`. Then `ASTDumper` would need a forwarding API for it. Anyway, your argument also applies to `JSONNodeDumper`, but you put that in a header file. That was sane. We should move `ASTDumper` to a header similarly. (Perhaps a rename of the class would help though?) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61835/new/ https://reviews.llvm.org/D61835 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
