NFC: Move dump of type nodes to NodeDumper

[steveire]

Format

[steveire]

Update

[aaron.ballman]

Mostly looks good (a few minor nits), but I did have a design question about whether we should prefer calling a Visit method for type hierarchies instead of reimplementing the functionality.

lib/AST/ASTDumper.cpp

145

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?

164

Why this approach as opposed to deferring to VisitFunctionType() as before?

lib/AST/TextNodeDumper.cpp

14–16

Remove newline

960

It'd be nice to remove this use of auto.

971

It'd be nice to remove this use of auto.

1047

It'd be nice to remove this use of auto.

[steveire]

lib/AST/ASTDumper.cpp

145

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.

[aaron.ballman]

lib/AST/ASTDumper.cpp

145

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).

[steveire]

Nits

[aaron.ballman]

LGTM aside from auto nits.

include/clang/AST/TextNodeDumper.h

 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDumperUtils.h"
 #include "clang/AST/AttrVisitor.h"
 #include "clang/AST/CommentCommandTraits.h"
 #include "clang/AST/CommentVisitor.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/AST/TemplateArgumentVisitor.h"
+#include "clang/AST/TypeVisitor.h"
 
 namespace clang {
 
 class TextTreeStructure {
   raw_ostream &OS;
   const bool ShowColors;
 
   /// Pending[i] is an action to dump an entity at level i.

 };
 
 class TextNodeDumper
     : public TextTreeStructure,
       public comments::ConstCommentVisitor<TextNodeDumper, void,
                                            const comments::FullComment *>,
       public ConstAttrVisitor<TextNodeDumper>,
       public ConstTemplateArgumentVisitor<TextNodeDumper>,
-      public ConstStmtVisitor<TextNodeDumper> {
+      public ConstStmtVisitor<TextNodeDumper>,
+      public TypeVisitor<TextNodeDumper> {
   raw_ostream &OS;
   const bool ShowColors;
 
   /// Keep track of the last location we print out so that we can
   /// print out deltas from then on out.
   const char *LastLocFilename = "";
   unsigned LastLocLine = ~0U;
 
#include "clang/AST/AttrTextNodeDump.inc"
   void VisitObjCBoxedExpr(const ObjCBoxedExpr *Node);
   void VisitObjCSelectorExpr(const ObjCSelectorExpr *Node);
   void VisitObjCProtocolExpr(const ObjCProtocolExpr *Node);
   void VisitObjCPropertyRefExpr(const ObjCPropertyRefExpr *Node);
   void VisitObjCSubscriptRefExpr(const ObjCSubscriptRefExpr *Node);
   void VisitObjCIvarRefExpr(const ObjCIvarRefExpr *Node);
   void VisitObjCBoolLiteralExpr(const ObjCBoolLiteralExpr *Node);
 
+  void VisitRValueReferenceType(const ReferenceType *T);
+  void VisitArrayType(const ArrayType *T);
+  void VisitConstantArrayType(const ConstantArrayType *T);
+  void VisitVariableArrayType(const VariableArrayType *T);
+  void VisitDependentSizedArrayType(const DependentSizedArrayType *T);
+  void VisitDependentSizedExtVectorType(const DependentSizedExtVectorType *T);
+  void VisitVectorType(const VectorType *T);
+  void VisitFunctionType(const FunctionType *T);
+  void VisitFunctionProtoType(const FunctionProtoType *T);
+  void VisitUnresolvedUsingType(const UnresolvedUsingType *T);
+  void VisitTypedefType(const TypedefType *T);
+  void VisitUnaryTransformType(const UnaryTransformType *T);
+  void VisitTagType(const TagType *T);
+  void VisitTemplateTypeParmType(const TemplateTypeParmType *T);
+  void VisitAutoType(const AutoType *T);
+  void VisitTemplateSpecializationType(const TemplateSpecializationType *T);
+  void VisitInjectedClassNameType(const InjectedClassNameType *T);
+  void VisitObjCInterfaceType(const ObjCInterfaceType *T);
+  void VisitPackExpansionType(const PackExpansionType *T);
+
 private:
   void dumpCXXTemporary(const CXXTemporary *Temporary);
 };
 
 } // namespace clang
 
 #endif // LLVM_CLANG_AST_TEXTNODEDUMPER_H

lib/AST/ASTDumper.cpp

void VisitPointerType(const PointerType *T) {
       dumpTypeAsChild(T->getPointeeType());
     }
     void VisitBlockPointerType(const BlockPointerType *T) {
       dumpTypeAsChild(T->getPointeeType());
     }
     void VisitReferenceType(const ReferenceType *T) {
       dumpTypeAsChild(T->getPointeeType());
     }
-    void VisitRValueReferenceType(const ReferenceType *T) {
-      if (T->isSpelledAsLValue())
-        OS << " written as lvalue reference";
-      VisitReferenceType(T);
-    }
     void VisitMemberPointerType(const MemberPointerType *T) {
       dumpTypeAsChild(T->getClass());
       dumpTypeAsChild(T->getPointeeType());
     }
     void VisitArrayType(const ArrayType *T) {
-      switch (T->getSizeModifier()) {
-        case ArrayType::Normal: break;
-        case ArrayType::Static: OS << " static"; break;
-        case ArrayType::Star: OS << " *"; break;
-      }
-      OS << " " << T->getIndexTypeQualifiers().getAsString();
       dumpTypeAsChild(T->getElementType());
     }
-    void VisitConstantArrayType(const ConstantArrayType *T) {
-      OS << " " << T->getSize();
-      VisitArrayType(T);
-    }
     void VisitVariableArrayType(const VariableArrayType *T) {
-      OS << " ";
-      NodeDumper.dumpSourceRange(T->getBracketsRange());
       VisitArrayType(T);
       dumpStmt(T->getSizeExpr());
     }
     void VisitDependentSizedArrayType(const DependentSizedArrayType *T) {
-      switch (T->getSizeModifier()) {
-        case ArrayType::Normal: break;
-        case ArrayType::Static: OS << " static"; break;
-        case ArrayType::Star: OS << " *"; break;
-      }
-      OS << " " << T->getIndexTypeQualifiers().getAsString();
-      OS << " ";
-      NodeDumper.dumpSourceRange(T->getBracketsRange());
       dumpTypeAsChild(T->getElementType());
       dumpStmt(T->getSizeExpr());
     }
     void VisitDependentSizedExtVectorType(
         const DependentSizedExtVectorType *T) {
-      OS << " ";
-      NodeDumper.dumpLocation(T->getAttributeLoc());
       dumpTypeAsChild(T->getElementType());
       dumpStmt(T->getSizeExpr());
     }
     void VisitVectorType(const VectorType *T) {
-      switch (T->getVectorKind()) {
-        case VectorType::GenericVector: break;
-        case VectorType::AltiVecVector: OS << " altivec"; break;
-        case VectorType::AltiVecPixel: OS << " altivec pixel"; break;
-        case VectorType::AltiVecBool: OS << " altivec bool"; break;
-        case VectorType::NeonVector: OS << " neon"; break;
-        case VectorType::NeonPolyVector: OS << " neon poly"; break;
-      }
-      OS << " " << T->getNumElements();
       dumpTypeAsChild(T->getElementType());
     }
     void VisitFunctionType(const FunctionType *T) {
-      auto EI = T->getExtInfo();
-      if (EI.getNoReturn()) OS << " noreturn";
-      if (EI.getProducesResult()) OS << " produces_result";
-      if (EI.getHasRegParm()) OS << " regparm " << EI.getRegParm();
-      OS << " " << FunctionType::getNameForCallConv(EI.getCC());
       dumpTypeAsChild(T->getReturnType());
     }
     void VisitFunctionProtoType(const FunctionProtoType *T) {
-      auto EPI = T->getExtProtoInfo();
-      if (EPI.HasTrailingReturn) OS << " trailing_return";
-
-      if (!T->getTypeQuals().empty())
-        OS << " " << T->getTypeQuals().getAsString();
-
-      switch (EPI.RefQualifier) {
-        case RQ_None: break;
-        case RQ_LValue: OS << " &"; break;
-        case RQ_RValue: OS << " &&"; break;
-      }
-      // FIXME: Exception specification.
-      // FIXME: Consumed parameters.
       VisitFunctionType(T);
       for (QualType PT : T->getParamTypes())
         dumpTypeAsChild(PT);
-      if (EPI.Variadic)
+      if (T->getExtProtoInfo().Variadic)
         dumpChild([=] { OS << "..."; });
     }
-    void VisitUnresolvedUsingType(const UnresolvedUsingType *T) {
-      NodeDumper.dumpDeclRef(T->getDecl());
-    }
-    void VisitTypedefType(const TypedefType *T) {
-      NodeDumper.dumpDeclRef(T->getDecl());
-    }
     void VisitTypeOfExprType(const TypeOfExprType *T) {
       dumpStmt(T->getUnderlyingExpr());
     }
     void VisitDecltypeType(const DecltypeType *T) {
       dumpStmt(T->getUnderlyingExpr());
     }
     void VisitUnaryTransformType(const UnaryTransformType *T) {
-      switch (T->getUTTKind()) {
-      case UnaryTransformType::EnumUnderlyingType:
-        OS << " underlying_type";
-        break;
-      }
       dumpTypeAsChild(T->getBaseType());
     }
-    void VisitTagType(const TagType *T) {
-      NodeDumper.dumpDeclRef(T->getDecl());
-    }
     void VisitAttributedType(const AttributedType *T) {
       // FIXME: AttrKind
       dumpTypeAsChild(T->getModifiedType());
     }
-    void VisitTemplateTypeParmType(const TemplateTypeParmType *T) {
-      OS << " depth " << T->getDepth() << " index " << T->getIndex();
-      if (T->isParameterPack()) OS << " pack";
-      NodeDumper.dumpDeclRef(T->getDecl());
-    }
     void VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *T) {
       dumpTypeAsChild(T->getReplacedParameter());
     }
     void VisitSubstTemplateTypeParmPackType(
         const SubstTemplateTypeParmPackType *T) {
       dumpTypeAsChild(T->getReplacedParameter());
       dumpTemplateArgument(T->getArgumentPack());
     }
-    void VisitAutoType(const AutoType *T) {
-      if (T->isDecltypeAuto()) OS << " decltype(auto)";
-      if (!T->isDeduced())
-        OS << " undeduced";
-    }
     void VisitTemplateSpecializationType(const TemplateSpecializationType *T) {
-      if (T->isTypeAlias()) OS << " alias";
-      OS << " "; T->getTemplateName().dump(OS);
       for (auto &Arg : *T)
         dumpTemplateArgument(Arg);
       if (T->isTypeAlias())
         dumpTypeAsChild(T->getAliasedType());
     }
-    void VisitInjectedClassNameType(const InjectedClassNameType *T) {
-      NodeDumper.dumpDeclRef(T->getDecl());
-    }
-    void VisitObjCInterfaceType(const ObjCInterfaceType *T) {
-      NodeDumper.dumpDeclRef(T->getDecl());
-    }
     void VisitObjCObjectPointerType(const ObjCObjectPointerType *T) {
       dumpTypeAsChild(T->getPointeeType());
     }
     void VisitAtomicType(const AtomicType *T) {
       dumpTypeAsChild(T->getValueType());
     }
     void VisitPipeType(const PipeType *T) {
       dumpTypeAsChild(T->getElementType());
     }
     void VisitAdjustedType(const AdjustedType *T) {
       dumpTypeAsChild(T->getOriginalType());
     }
     void VisitPackExpansionType(const PackExpansionType *T) {
-      if (auto N = T->getNumExpansions()) OS << " expansions " << *N;
       if (!T->isSugared())
         dumpTypeAsChild(T->getPattern());
     }
     // FIXME: ElaboratedType, DependentNameType,
     // DependentTemplateSpecializationType, ObjCObjectType
 
     // Decls
     void VisitLabelDecl(const LabelDecl *D);

lib/AST/TextNodeDumper.cpp

 //===--- TextNodeDumper.cpp - Printing of AST nodes -----------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements AST dumping of components of individual AST nodes.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/TextNodeDumper.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/LocInfoType.h"
 
 using namespace clang;
 
 TextNodeDumper::TextNodeDumper(raw_ostream &OS, bool ShowColors,
                                const SourceManager *SM,
                                const PrintingPolicy &PrintPolicy,
                                const comments::CommandTraits *Traits)
     : TextTreeStructure(OS, ShowColors), OS(OS), ShowColors(ShowColors), SM(SM),
else if (T->isInstantiationDependentType())
     OS << " instantiation_dependent";
 
   if (T->isVariablyModifiedType())
     OS << " variably_modified";
   if (T->containsUnexpandedParameterPack())
     OS << " contains_unexpanded_pack";
   if (T->isFromAST())
     OS << " imported";
+
+  TypeVisitor<TextNodeDumper>::Visit(T);
 }
 
 void TextNodeDumper::Visit(QualType T) {
   OS << "QualType";
   dumpPointer(T.getAsOpaquePtr());
   OS << " ";
   dumpBareType(T, false);
   OS << " " << T.split().Quals.getAsString();
if (Node->setAtIndexMethodDecl())
     Node->setAtIndexMethodDecl()->getSelector().print(OS);
   else
     OS << "(null)";
 }
 
 void TextNodeDumper::VisitObjCBoolLiteralExpr(const ObjCBoolLiteralExpr *Node) {
   OS << " " << (Node->getValue() ? "__objc_yes" : "__objc_no");
 }
+
+void TextNodeDumper::VisitRValueReferenceType(const ReferenceType *T) {
+  if (T->isSpelledAsLValue())
+    OS << " written as lvalue reference";
+}
+
+void TextNodeDumper::VisitArrayType(const ArrayType *T) {
+  switch (T->getSizeModifier()) {
+  case ArrayType::Normal:
+    break;
+  case ArrayType::Static:
+    OS << " static";
+    break;
+  case ArrayType::Star:
+    OS << " *";
+    break;
+  }
+  OS << " " << T->getIndexTypeQualifiers().getAsString();
+}
+
+void TextNodeDumper::VisitConstantArrayType(const ConstantArrayType *T) {
+  OS << " " << T->getSize();
+  VisitArrayType(T);
+}
+
+void TextNodeDumper::VisitVariableArrayType(const VariableArrayType *T) {
+  OS << " ";
+  dumpSourceRange(T->getBracketsRange());
+  VisitArrayType(T);
+}
+
+void TextNodeDumper::VisitDependentSizedArrayType(
+    const DependentSizedArrayType *T) {
+  VisitArrayType(T);
+  OS << " ";
+  dumpSourceRange(T->getBracketsRange());
+}
+
+void TextNodeDumper::VisitDependentSizedExtVectorType(
+    const DependentSizedExtVectorType *T) {
+  OS << " ";
+  dumpLocation(T->getAttributeLoc());
+}
+
+void TextNodeDumper::VisitVectorType(const VectorType *T) {
+  switch (T->getVectorKind()) {
+  case VectorType::GenericVector:
+    break;
+  case VectorType::AltiVecVector:
+    OS << " altivec";
+    break;
+  case VectorType::AltiVecPixel:
+    OS << " altivec pixel";
+    break;
+  case VectorType::AltiVecBool:
+    OS << " altivec bool";
+    break;
+  case VectorType::NeonVector:
+    OS << " neon";
+    break;
+  case VectorType::NeonPolyVector:
+    OS << " neon poly";
+    break;
+  }
+  OS << " " << T->getNumElements();
+}
+
+void TextNodeDumper::VisitFunctionType(const FunctionType *T) {
+  auto EI = T->getExtInfo();
+  if (EI.getNoReturn())
+    OS << " noreturn";
+  if (EI.getProducesResult())
+    OS << " produces_result";
+  if (EI.getHasRegParm())
+    OS << " regparm " << EI.getRegParm();
+  OS << " " << FunctionType::getNameForCallConv(EI.getCC());
+}
+
+void TextNodeDumper::VisitFunctionProtoType(const FunctionProtoType *T) {
+  auto EPI = T->getExtProtoInfo();
+  if (EPI.HasTrailingReturn)
+    OS << " trailing_return";
+  if (T->isConst())
+    OS << " const";
+  if (T->isVolatile())
+    OS << " volatile";
+  if (T->isRestrict())
+    OS << " restrict";
+  switch (EPI.RefQualifier) {
+  case RQ_None:
+    break;
+  case RQ_LValue:
+    OS << " &";
+    break;
+  case RQ_RValue:
+    OS << " &&";
+    break;
+  }
+  // FIXME: Exception specification.
+  // FIXME: Consumed parameters.
+  VisitFunctionType(T);
+}
+
+void TextNodeDumper::VisitUnresolvedUsingType(const UnresolvedUsingType *T) {
+  dumpDeclRef(T->getDecl());
+}
+
+void TextNodeDumper::VisitTypedefType(const TypedefType *T) {
+  dumpDeclRef(T->getDecl());
+}
+
+void TextNodeDumper::VisitUnaryTransformType(const UnaryTransformType *T) {
+  switch (T->getUTTKind()) {
+  case UnaryTransformType::EnumUnderlyingType:
+    OS << " underlying_type";
+    break;
+  }
+}
+
+void TextNodeDumper::VisitTagType(const TagType *T) {
+  dumpDeclRef(T->getDecl());
+}
+
+void TextNodeDumper::VisitTemplateTypeParmType(const TemplateTypeParmType *T) {
+  OS << " depth " << T->getDepth() << " index " << T->getIndex();
+  if (T->isParameterPack())
+    OS << " pack";
+  dumpDeclRef(T->getDecl());
+}
+
+void TextNodeDumper::VisitAutoType(const AutoType *T) {
+  if (T->isDecltypeAuto())
+    OS << " decltype(auto)";
+  if (!T->isDeduced())
+    OS << " undeduced";
+}
+
+void TextNodeDumper::VisitTemplateSpecializationType(
+    const TemplateSpecializationType *T) {
+  if (T->isTypeAlias())
+    OS << " alias";
+  OS << " ";
+  T->getTemplateName().dump(OS);
+}
+
+void TextNodeDumper::VisitInjectedClassNameType(
+    const InjectedClassNameType *T) {
+  dumpDeclRef(T->getDecl());
+}
+
+void TextNodeDumper::VisitObjCInterfaceType(const ObjCInterfaceType *T) {
+  dumpDeclRef(T->getDecl());
+}
+
+void TextNodeDumper::VisitPackExpansionType(const PackExpansionType *T) {
+  if (auto N = T->getNumExpansions())
+    OS << " expansions " << *N;
+}