[PATCH] D61835: Extract ASTDumper to a header file

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 16 11:54:43 PDT 2019


aaron.ballman added a comment.

In D61835#1505202 <https://reviews.llvm.org/D61835#1505202>, @steveire wrote:

> In D61835#1505151 <https://reviews.llvm.org/D61835#1505151>, @aaron.ballman wrote:
>
> > In D61835#1505048 <https://reviews.llvm.org/D61835#1505048>, @steveire wrote:
> >
> > > The users of the follow-up patch https://reviews.llvm.org/D61837#change-x5mxz9Lpijjs need that 'correctness', but also need the public API from `ASTNodeTraverser` on the instance. (That patch also extends the public API for users).
> >
> >
> > I don't see anything in the follow-up patch that actually uses the `ASTDumper` class though, so I'm still a bit confused.
>
>
> Maybe my wording was confusing. I'll try again:
>
> 1. Anyone who wants traversal in the same way that dumping is done must use an instance of the `ASTDumper` class. Is that much clear? Does my previous response clarify why that is the case?


Yup, on the same page there for the most part. Is the "anyone" you refer to someone developing clang (or clang's tools), or third party? I've been envisioning clang development.

> 2. The follow-up patch https://reviews.llvm.org/D61837#change-x5mxz9Lpijjs adds API to ASTNodeTraverser.

Yup, definitely can see that.

> 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? e.g., can they be given a reference to the base class rather than the derived class? For instance, `Decl::dump()` creates an `ASTDumper` instance, but to initiate the traversal, it calls `Visit()` from the base class.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61835





More information about the cfe-commits mailing list