[PATCH] D56642: NFC: Move dump of type nodes to NodeDumper

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: lib/AST/ASTDumper.cpp:145
     void VisitVariableArrayType(const VariableArrayType *T) {
-      OS << " ";
-      NodeDumper.dumpSourceRange(T->getBracketsRange());
-      VisitArrayType(T);
+      dumpTypeAsChild(T->getElementType());
       dumpStmt(T->getSizeExpr());
----------------
steveire wrote:
> aaron.ballman wrote:
> > Why this approach instead of deferring to `VisitArrayType()` as before? I prefer calling the Visit function rather than reimplementing the functionality because we may decide to later improve the base class printing and expect subclasses to automatically pick that up. WDYT?
> I think it's more clear to read what each visit method does, but I don't feel strongly about it. I can change this if you do.
Given that there's an established type hierarchy, I prefer calling the `Visit()` function for the superclass rather than reimplementing. The extra indirection wasn't too annoying with the old code, so it's unlikely to be too problematic with the new code (and if it is, then we'll have reason to change).


Repository:
  rC Clang

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

https://reviews.llvm.org/D56642