I’ve been looking at the code in ASTDumper.cpp and noticed that most of it basically uses one of three approaches:
Some of it uses the ASTDumper’s own classes, like PrintExpr and PrintStmt
Some of it uses an external ASTPrinter class that implements some printing logic used in other parts of the compiler
And some of it just prints directly to the output stream, with (seemingly) no centralized logic
While I imagine this is just the result of several unrelated changes throughout the compiler’s development, I wanted to ask, is there a specific motivation behind these decisions? And is there a plan (or a desire, perhaps) to eventually simplify and unify these approaches?
ASTPrinter needs to print a source-like view of code rather than a tree structure. (This is extra important now with the work on textual interfaces.) ASTDumper usually uses this when referring to some other AST node that isn't really a child, like the underlying type of a typealias.
PrintExpr and PrintStmt are subclasses of ASTVisitor; keeping them separate helps keep the code clean by separating concerns. (At least, that's the theory, anyway.)
If none of this functionality is needed, there's no reason not to print directly to the output stream.
Anything that makes the code cleaner is probably a welcome patch, but I wouldn't say the current division of labor is something we consider a problem.
What about methods like Stmt::print and Stmt::dump? It seems to me like they’re a way for other parts of the compiler to print AST nodes in a tree structure, is that about right?
dump() is 99% of the time present to make debugging easier, since a real dumper would usually want to control the output stream rather than just using stderr. print(…) seems to be a way to hide the details of PrintExpr, but it might make more sense if that went consistently through ASTPrinter (as noted by FIXME in one of the APIs). The convenience of having a print(…) method right on an AST node type might still be worth it, though.
Aside: we also haven't really put effort into pretty-printing Exprs and Stmts, only Decls.
Hey @jrose, I submitted a pull request (#19985) on this issue. It's my first, so it might not be quite right, but I'd love some feedback if you have the time.