Improves pretty printing of variable types in llvm-pdbdump

[amccarth]

Summary

• Adds support for pointers to arrays, which was missing
• Adds some tests
• Makes one test more robust in the face of unpredictable ordering in the generated PDB
• Improves consistency of const and volatile qualifiers
• Eliminates non-composable special case code for arrays and function by using a more general recursive approach
• Has a hack for getting the calling convention into the right spot for pointer-to-functions

Given the rapid changes happenning in llvm-pdbdump, this may be difficult to merge.

[zturner]

lgtm, only some minor nits.

llvm/tools/llvm-pdbdump/PrettyVariableDumper.cpp

154 ↗

(On Diff #94572)

You'll probably need to do a merge here, as I changed the syntax for this line.

159 ↗

(On Diff #94572)

here I'd suggest just using isa<PDBSymbolTypeArray> since we don't actually need the derived class.

170–172 ↗

(On Diff #94572)

I wonder if it's worth asserting here. You can still return if it's null, but it seems like a bad PDB file at the same time.

173–174 ↗

(On Diff #94572)

isa<> again.

[amccarth]

Thanks for the review, especially for the heads up on the merge points.

llvm/tools/llvm-pdbdump/PrettyVariableDumper.cpp

170–172 ↗

(On Diff #94572)

Agreed. I was following a pattern I saw in other parts of the code, but the assert makes sense. I've also added the for ElementType in the array cases.

llvm/trunk/include/llvm/DebugInfo/PDB/PDBSymDumper.h

public:
   virtual void dump(const PDBSymbolTypePointer &Symbol);
   virtual void dump(const PDBSymbolTypeTypedef &Symbol);
   virtual void dump(const PDBSymbolTypeUDT &Symbol);
   virtual void dump(const PDBSymbolTypeVTable &Symbol);
   virtual void dump(const PDBSymbolTypeVTableShape &Symbol);
   virtual void dump(const PDBSymbolUnknown &Symbol);
   virtual void dump(const PDBSymbolUsingNamespace &Symbol);
 
+  virtual void dumpRight(const PDBSymbolTypeArray &Symbol) {}
+  virtual void dumpRight(const PDBSymbolTypeBaseClass &Symbol) {}
+  virtual void dumpRight(const PDBSymbolTypeBuiltin &Symbol) {}
+  virtual void dumpRight(const PDBSymbolTypeCustom &Symbol) {}
+  virtual void dumpRight(const PDBSymbolTypeDimension &Symbol) {}
+  virtual void dumpRight(const PDBSymbolTypeEnum &Symbol) {}
+  virtual void dumpRight(const PDBSymbolTypeFriend &Symbol) {}
+  virtual void dumpRight(const PDBSymbolTypeFunctionArg &Symbol) {}
+  virtual void dumpRight(const PDBSymbolTypeFunctionSig &Symbol) {}
+  virtual void dumpRight(const PDBSymbolTypeManaged &Symbol) {}
+  virtual void dumpRight(const PDBSymbolTypePointer &Symbol) {}
+  virtual void dumpRight(const PDBSymbolTypeTypedef &Symbol) {}
+  virtual void dumpRight(const PDBSymbolTypeUDT &Symbol) {}
+  virtual void dumpRight(const PDBSymbolTypeVTable &Symbol) {}
+  virtual void dumpRight(const PDBSymbolTypeVTableShape &Symbol) {}
+
 private:
   bool RequireImpl;
 };
 }
 }
 
 #endif

llvm/trunk/include/llvm/DebugInfo/PDB/PDBSymbol.h

public:
 
   virtual ~PDBSymbol();
 
   /// Dumps the contents of a symbol a raw_ostream.  By default this will just
   /// call dump() on the underlying RawSymbol, which allows us to discover
   /// unknown properties, but individual implementations of PDBSymbol may
   /// override the behavior to only dump known fields.
   virtual void dump(PDBSymDumper &Dumper) const = 0;
+
+  /// For certain PDBSymbolTypes, dumps additional information for the type that
+  /// normally goes on the right side of the symbol.
+  virtual void dumpRight(PDBSymDumper &Dumper) const {}
+
   void defaultDump(raw_ostream &OS, int Indent) const;
   void dumpProperties() const;
   void dumpChildStats() const;
 
   PDB_SymType getSymTag() const;
   uint32_t getSymIndexId() const;
 
   template <typename T> std::unique_ptr<T> findOneChild() const {

llvm/trunk/include/llvm/DebugInfo/PDB/PDBSymbolTypeArray.h

 class PDBSymbolTypeArray : public PDBSymbol {
 public:
   PDBSymbolTypeArray(const IPDBSession &PDBSession,
                      std::unique_ptr<IPDBRawSymbol> ArrayTypeSymbol);
 
   DECLARE_PDB_SYMBOL_CONCRETE_TYPE(PDB_SymType::ArrayType)
 
   void dump(PDBSymDumper &Dumper) const override;
+  void dumpRight(PDBSymDumper &Dumper) const override;
 
   FORWARD_SYMBOL_ID_METHOD(getArrayIndexType)
   FORWARD_SYMBOL_METHOD(isConstType)
   FORWARD_SYMBOL_METHOD(getCount)
   FORWARD_SYMBOL_METHOD(getLength)
   FORWARD_SYMBOL_ID_METHOD(getLexicalParent)
   FORWARD_SYMBOL_METHOD(getRank)
   FORWARD_SYMBOL_ID_METHOD_WITH_NAME(getType, getElementType)
   FORWARD_SYMBOL_METHOD(isUnalignedType)
   FORWARD_SYMBOL_METHOD(isVolatileType)
 };
 
 } // namespace llvm
 }
 
 #endif // LLVM_DEBUGINFO_PDB_PDBSYMBOLTYPEARRAY_H

llvm/trunk/include/llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h

public:
   PDBSymbolTypeFunctionSig(const IPDBSession &PDBSession,
                            std::unique_ptr<IPDBRawSymbol> Symbol);
 
   DECLARE_PDB_SYMBOL_CONCRETE_TYPE(PDB_SymType::FunctionSig)
 
   std::unique_ptr<IPDBEnumSymbols> getArguments() const;
 
   void dump(PDBSymDumper &Dumper) const override;
+  void dumpRight(PDBSymDumper &Dumper) const override;
   void dumpArgList(raw_ostream &OS) const;
 
   FORWARD_SYMBOL_METHOD(getCallingConvention)
   FORWARD_SYMBOL_ID_METHOD(getClassParent)
   FORWARD_SYMBOL_ID_METHOD(getUnmodifiedType)
   FORWARD_SYMBOL_METHOD(isConstType)
   FORWARD_SYMBOL_METHOD(getCount)
   FORWARD_SYMBOL_ID_METHOD(getLexicalParent)

llvm/trunk/include/llvm/DebugInfo/PDB/PDBSymbolTypePointer.h

 class PDBSymbolTypePointer : public PDBSymbol {
 public:
   PDBSymbolTypePointer(const IPDBSession &PDBSession,
                        std::unique_ptr<IPDBRawSymbol> Symbol);
 
   DECLARE_PDB_SYMBOL_CONCRETE_TYPE(PDB_SymType::PointerType)
 
   void dump(PDBSymDumper &Dumper) const override;
+  void dumpRight(PDBSymDumper &Dumper) const override;
 
   FORWARD_SYMBOL_METHOD(isConstType)
   FORWARD_SYMBOL_METHOD(getLength)
   FORWARD_SYMBOL_ID_METHOD(getLexicalParent)
   FORWARD_SYMBOL_METHOD(isReference)
   FORWARD_SYMBOL_ID_METHOD_WITH_NAME(getType, getPointeeType)
   FORWARD_SYMBOL_METHOD(isUnalignedType)
   FORWARD_SYMBOL_METHOD(isVolatileType)
 };
 
 } // namespace llvm
 }
 
 #endif // LLVM_DEBUGINFO_PDB_PDBSYMBOLTYPEPOINTER_H

llvm/trunk/lib/DebugInfo/PDB/PDBSymbolTypeArray.cpp

PDBSymbolTypeArray::PDBSymbolTypeArray(const IPDBSession &PDBSession,
                                        std::unique_ptr<IPDBRawSymbol> Symbol)
     : PDBSymbol(PDBSession, std::move(Symbol)) {
   assert(RawSymbol->getSymTag() == PDB_SymType::ArrayType);
 }
 
 void PDBSymbolTypeArray::dump(PDBSymDumper &Dumper) const {
   Dumper.dump(*this);
 }
+
+void PDBSymbolTypeArray::dumpRight(PDBSymDumper &Dumper) const {
+  Dumper.dumpRight(*this);
+}

llvm/trunk/lib/DebugInfo/PDB/PDBSymbolTypeFunctionSig.cpp

 std::unique_ptr<IPDBEnumSymbols>
 PDBSymbolTypeFunctionSig::getArguments() const {
   return llvm::make_unique<FunctionArgEnumerator>(Session, *this);
 }
 
 void PDBSymbolTypeFunctionSig::dump(PDBSymDumper &Dumper) const {
   Dumper.dump(*this);
 }
+
+void PDBSymbolTypeFunctionSig::dumpRight(PDBSymDumper &Dumper) const {
+  Dumper.dumpRight(*this);
+}

llvm/trunk/lib/DebugInfo/PDB/PDBSymbolTypePointer.cpp

PDBSymbolTypePointer::PDBSymbolTypePointer(
     const IPDBSession &PDBSession, std::unique_ptr<IPDBRawSymbol> Symbol)
     : PDBSymbol(PDBSession, std::move(Symbol)) {
   assert(RawSymbol->getSymTag() == PDB_SymType::PointerType);
 }
 
 void PDBSymbolTypePointer::dump(PDBSymDumper &Dumper) const {
   Dumper.dump(*this);
 }
+
+void PDBSymbolTypePointer::dumpRight(PDBSymDumper &Dumper) const {
+  Dumper.dumpRight(*this);
+}

llvm/trunk/test/DebugInfo/PDB/DIA/pdbdump-linenumbers.test

+; RUN: llvm-pdbdump pretty -lines %p/../Inputs/symbolformat.pdb | FileCheck --check-prefix=LINE_NUMS_FPO %s
 ; RUN: llvm-pdbdump pretty -lines %p/../Inputs/symbolformat.pdb | FileCheck --check-prefix=LINE_NUMS %s
 
-; LINE_NUMS: llvm\test\debuginfo\pdb\inputs\symbolformat-fpo.cpp
-; LINE_NUMS: Line 5, Address: [0x000011a0 - 0x000011a5] (6 bytes)
-; LINE_NUMS: Line 6, Address: [0x000011a6 - 0x000011a6] (1 bytes)
+; LINE_NUMS_FPO: llvm\test\debuginfo\pdb\inputs\symbolformat-fpo.cpp
+; LINE_NUMS_FPO: Line 5, Address: [0x000011a0 - 0x000011a5] (6 bytes)
+; LINE_NUMS_FPO: Line 6, Address: [0x000011a6 - 0x000011a6] (1 bytes)
+
 ; LINE_NUMS: llvm\test\debuginfo\pdb\inputs\symbolformat.cpp
 ; LINE_NUMS: Line 6, Address: [0x00001060 - 0x00001066] (7 bytes)
-; LINE_NUMS: Line 72, Address: [0x000010d0 - 0x000010d1] (2 bytes)
-; LINE_NUMS: Line 73, Address: [0x000010d2 - 0x000010d5] (4 bytes)
+; LINE_NUMS: Line 80, Address: [0x000010d0 - 0x000010d1] (2 bytes)
+; LINE_NUMS: Line 81, Address: [0x000010d2 - 0x000010d5] (4 bytes)
 ; LINE_NUMS: Line 28, Address: [0x00001170 - 0x0000117a] (11 bytes)
 ; LINE_NUMS: Line 21, Address: [0x00001180 - 0x0000118a] (11 bytes)
-; LINE_NUMS: Line 20, Address: [0x00001190 - 0x0000119a] (11 bytes)
+; LINE_NUMS: Line 20, Address: [0x00001190 - 0x0000119a] (11 bytes)
 

llvm/trunk/test/DebugInfo/PDB/DIA/pdbdump-symbol-format.test

+; RUN: llvm-pdbdump pretty -symbols %p/../Inputs/symbolformat.pdb | FileCheck --check-prefix=SYM_FORMAT_FPO %s
 ; RUN: llvm-pdbdump pretty -symbols %p/../Inputs/symbolformat.pdb | FileCheck --check-prefix=SYM_FORMAT %s
 ; RUN: llvm-pdbdump pretty -types %p/../Inputs/symbolformat.pdb > %t.types
 ; RUN: FileCheck --check-prefix=TYPES_FORMAT %s < %t.types
 ; RUN: FileCheck --check-prefix=TYPES_1 %s < %t.types
 ; RUN: FileCheck --check-prefix=TYPES_2 %s < %t.types
 ; RUN: llvm-pdbdump pretty -types %p/../Inputs/symbolformat.pdb | FileCheck --check-prefix=TYPES_FORMAT %s
 ; RUN: llvm-pdbdump pretty -globals %p/../Inputs/symbolformat.pdb | FileCheck --check-prefix=GLOBALS %s
 
 ; The format is func [0x<rva_start>+<prologue_length> - 0x<rva_end>-<epilogue_length>]
+; SYM_FORMAT_FPO: ---SYMBOLS---
+; SYM_FORMAT_FPO: symbolformat-fpo.obj
+; SYM_FORMAT-FPO: func [{{.*}}] (FPO) unsigned int __cdecl fpo_func(unsigned int n)
+
 ; SYM_FORMAT: ---SYMBOLS---
-; SYM_FORMAT: symbolformat-fpo.obj
-; SYM_FORMAT-DAG: func [{{.*}}] (FPO) unsigned int __cdecl fpo_func(unsigned int n)
 ; SYM_FORMAT: symbolformat.obj
 ; SYM_FORMAT-DAG: func [{{.*}}] (EBP) int __cdecl _purecall()
 ; SYM_FORMAT-DAG: func [{{.*}}] (EBP) int __cdecl main(int argc, char** argv)
 ; SYM_FORMAT-DAG: func [{{.*}}] (EBP) void A::A()
 ; SYM_FORMAT-DAG: func [{{.*}}] (EBP) void B::B()
 ; SYM_FORMAT-DAG: thunk [{{.*}}] (Pcode) B::`vcall'{0}'
 ; SYM_FORMAT-DAG: func [{{.*}}] (EBP) virtual void B::PureFunc()
 ; SYM_FORMAT-DAG: func [{{.*}}] (EBP) void A::RegularFunc()

 ; TYPES_2: data +0x0c int m_int
 ; TYPES_2: data +0x10 unsigned int m_unsigned
 ; TYPES_2: data +0x14 long m_long
 ; TYPES_2: data +0x18 unsigned long m_unsigned_long
 ; TYPES_2: data +0x20 __int64 m_int64
 ; TYPES_2: data +0x28 unsigned __int64 m_unsigned_int64
 ; TYPES_2: data +0x30 float m_float
 ; TYPES_2: data +0x38 double m_double
-; TYPES_2: data +0x40 void (__cdecl *m_pfn_2_args)(int, double)
+; TYPES_2: data +0x40 void (__cdecl * m_pfn_2_args)(int, double)
+; TYPES_2: data +0x44 int m_multidimensional_array[2][3]
 ; TYPES_2: }
 
 ; GLOBALS: ---GLOBALS---
 ; GLOBALS-DAG: func [{{.*}}] (FPO) unsigned int __cdecl fpo_func(unsigned int n)
 ; GLOBALS-DAG: data [{{.*}}] static void* g_global_pointer
 ; GLOBALS-DAG: data [{{.*}}] static int g_global_int
+; GLOBALS-DAG: data [{{.*}}] static int g_array[3]
+; GLOBALS-DAG: data [{{.*}}] static int (* g_pointer_to_array)[3]
+; GLOBALS-DAG: data [{{.*}}] static const int* g_pointer_to_const_int
+; GLOBALS-DAG: data int* const g_const_pointer_to_int = 0
+; GLOBALS-DAG: data const int* const g_const_pointer_to_const_int = 0

llvm/trunk/test/DebugInfo/PDB/Inputs/symbolformat.cpp

struct MemberTest {
   unsigned m_unsigned;
   long m_long;
   unsigned long m_unsigned_long;
   __int64 m_int64;
   unsigned __int64 m_unsigned_int64;
   float m_float;
   double m_double;
   void (*m_pfn_2_args)(int, double);
+  int m_multidimensional_array[2][3];
 };
 
 typedef int IntType;
 typedef A ClassAType;
 
 int g_global_int;
 void *g_global_pointer = nullptr;
 
+typedef int int_array[3];
+int_array g_array = { 1, 2, 3 };
+int_array *g_pointer_to_array = &g_array;
+const int *g_pointer_to_const_int = nullptr;
+int * const g_const_pointer_to_int = nullptr;
+const int * const g_const_pointer_to_const_int = nullptr;
+
 int main(int argc, char **argv) {
   // Force symbol references so the linker generates debug info
   B b;
   MemberTest members;
   auto PureAddr = &B::PureFunc;
   auto VirtualAddr = &A::PureFunc;
   auto RegularAddr = &A::RegularFunc;
   TestEnum Enum = Value;
   TestEnumClass EnumClass = TestEnumClass::Value10;
   IntType Int = 12;
   ClassAType *ClassA = &b;
   return 0;
 }

llvm/trunk/tools/llvm-pdbdump/PrettyBuiltinDumper.cpp

 
 using namespace llvm;
 using namespace llvm::pdb;
 
 BuiltinDumper::BuiltinDumper(LinePrinter &P)
     : PDBSymDumper(false), Printer(P) {}
 
 void BuiltinDumper::start(const PDBSymbolTypeBuiltin &Symbol) {
+  if (Symbol.isConstType())
+    WithColor(Printer, PDB_ColorItem::Keyword).get() << "const ";
+  if (Symbol.isVolatileType())
+    WithColor(Printer, PDB_ColorItem::Keyword).get() << "volatile ";
   WithColor(Printer, PDB_ColorItem::Type).get() << getTypeName(Symbol);
 }
 
 StringRef BuiltinDumper::getTypeName(const PDBSymbolTypeBuiltin &Symbol) {
   PDB_BuiltinType Type = Symbol.getBuiltinType();
   switch (Type) {
   case PDB_BuiltinType::Float:
     if (Symbol.getLength() == 4)

llvm/trunk/tools/llvm-pdbdump/PrettyVariableDumper.h

 class LinePrinter;
 
 class VariableDumper : public PDBSymDumper {
 public:
   VariableDumper(LinePrinter &P);
 
   void start(const PDBSymbolData &Var);
 
+  void dump(const PDBSymbolTypeArray &Symbol) override;
   void dump(const PDBSymbolTypeBuiltin &Symbol) override;
   void dump(const PDBSymbolTypeEnum &Symbol) override;
   void dump(const PDBSymbolTypeFunctionSig &Symbol) override;
   void dump(const PDBSymbolTypePointer &Symbol) override;
   void dump(const PDBSymbolTypeTypedef &Symbol) override;
   void dump(const PDBSymbolTypeUDT &Symbol) override;
 
+  void dumpRight(const PDBSymbolTypeArray &Symbol) override;
+  void dumpRight(const PDBSymbolTypeFunctionSig &Symbol) override;
+  void dumpRight(const PDBSymbolTypePointer &Symbol) override;
+
 private:
   void dumpSymbolTypeAndName(const PDBSymbol &Type, StringRef Name);
-  bool tryDumpFunctionPointer(const PDBSymbol &Type, StringRef Name);
 
   LinePrinter &Printer;
 };
 }
 }
 #endif

llvm/trunk/tools/llvm-pdbdump/PrettyVariableDumper.cpp

 //===- PrettyVariableDumper.cpp ---------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "PrettyVariableDumper.h"
 
 #include "LinePrinter.h"
 #include "PrettyBuiltinDumper.h"
 #include "PrettyFunctionDumper.h"
 #include "llvm-pdbdump.h"
 
+#include "llvm/DebugInfo/PDB/IPDBSession.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolData.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolFunc.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeArray.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypePointer.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeTypedef.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
+#include "llvm/DebugInfo/PDB/PDBTypes.h"
 
 #include "llvm/Support/Format.h"
 
 using namespace llvm;
+using namespace llvm::codeview;
 using namespace llvm::pdb;
 
 VariableDumper::VariableDumper(LinePrinter &P)
     : PDBSymDumper(true), Printer(P) {}
 
 void VariableDumper::start(const PDBSymbolData &Var) {
   if (Var.isCompilerGenerated() && opts::pretty::ExcludeCompilerGenerated)
     return;
case PDB_LocType::Static:
     WithColor(Printer, PDB_ColorItem::Keyword).get() << "static ";
     dumpSymbolTypeAndName(*VarType, Var.getName());
     break;
   case PDB_LocType::Constant:
     if (isa<PDBSymbolTypeEnum>(*VarType))
       break;
     Printer.NewLine();
     Printer << "data ";
-    WithColor(Printer, PDB_ColorItem::Keyword).get() << "const ";
     dumpSymbolTypeAndName(*VarType, Var.getName());
     Printer << " = ";
     WithColor(Printer, PDB_ColorItem::LiteralValue).get() << Var.getValue();
     break;
   case PDB_LocType::ThisRel:
     Printer.NewLine();
     Printer << "data ";
     WithColor(Printer, PDB_ColorItem::Offset).get()
default:
     Printer.NewLine();
     Printer << "data ";
     Printer << "unknown(" << LocType << ") ";
     WithColor(Printer, PDB_ColorItem::Identifier).get() << Var.getName();
     break;
   }
 }
 
+void VariableDumper::dump(const PDBSymbolTypeArray &Symbol) {
+  auto ElementType = Symbol.getElementType();
+  assert(ElementType);
+  if (!ElementType)
+    return;
+  ElementType->dump(*this);
+}
+
+void VariableDumper::dumpRight(const PDBSymbolTypeArray &Symbol) {
+  auto ElementType = Symbol.getElementType();
+  assert(ElementType);
+  if (!ElementType)
+    return;
+  Printer << '[' << Symbol.getCount() << ']';
+  ElementType->dumpRight(*this);
+}
+
 void VariableDumper::dump(const PDBSymbolTypeBuiltin &Symbol) {
   BuiltinDumper Dumper(Printer);
   Dumper.start(Symbol);
 }
 
 void VariableDumper::dump(const PDBSymbolTypeEnum &Symbol) {
   WithColor(Printer, PDB_ColorItem::Type).get() << Symbol.getName();
 }
 
-void VariableDumper::dump(const PDBSymbolTypeFunctionSig &Symbol) {}
+void VariableDumper::dump(const PDBSymbolTypeFunctionSig &Symbol) {
+  auto ReturnType = Symbol.getReturnType();
+  ReturnType->dump(*this);
+  Printer << " ";
+
+  uint32_t ClassParentId = Symbol.getClassParentId();
+  auto ClassParent =
+      Symbol.getSession().getConcreteSymbolById<PDBSymbolTypeUDT>(
+          ClassParentId);
+
+  if (ClassParent) {
+    WithColor(Printer, PDB_ColorItem::Identifier).get()
+      << ClassParent->getName();
+    Printer << "::";
+  }
+}
+
+void VariableDumper::dumpRight(const PDBSymbolTypeFunctionSig &Symbol) {
+  Printer << "(";
+  if (auto Arguments = Symbol.getArguments()) {
+    uint32_t Index = 0;
+    while (auto Arg = Arguments->getNext()) {
+      Arg->dump(*this);
+      if (++Index < Arguments->getChildCount())
+        Printer << ", ";
+    }
+  }
+  Printer << ")";
+
+  if (Symbol.isConstType())
+    WithColor(Printer, PDB_ColorItem::Keyword).get() << " const";
+  if (Symbol.isVolatileType())
+    WithColor(Printer, PDB_ColorItem::Keyword).get() << " volatile";
+}
 
 void VariableDumper::dump(const PDBSymbolTypePointer &Symbol) {
   auto PointeeType = Symbol.getPointeeType();
   if (!PointeeType)
     return;
-
-  if (auto Func = PointeeType->cast<PDBSymbolFunc>()) {
-    FunctionDumper NestedDumper(Printer);
-    FunctionDumper::PointerType Pointer =
-        Symbol.isReference() ? FunctionDumper::PointerType::Reference
-                             : FunctionDumper::PointerType::Pointer;
-    NestedDumper.start(*Func, Pointer);
-  } else {
-    if (Symbol.isConstType())
-      WithColor(Printer, PDB_ColorItem::Keyword).get() << "const ";
-    if (Symbol.isVolatileType())
-      WithColor(Printer, PDB_ColorItem::Keyword).get() << "volatile ";
-    PointeeType->dump(*this);
-    Printer << (Symbol.isReference() ? "&" : "*");
-  }
+  PointeeType->dump(*this);
+  if (auto Func = PointeeType->cast<PDBSymbolTypeFunctionSig>()) {
+    // A hack to get the calling convention in the right spot.
+    Printer << " (";
+    PDB_CallingConv CC = Func->getCallingConvention();
+    WithColor(Printer, PDB_ColorItem::Keyword).get() << CC << " ";
+  } else if (isa<PDBSymbolTypeArray>(PointeeType.get())) {
+    Printer << " (";
+  }
+  Printer << (Symbol.isReference() ? "&" : "*");
+    WithColor(Printer, PDB_ColorItem::Keyword).get() << " const ";
+    WithColor(Printer, PDB_ColorItem::Keyword).get() << " volatile ";
+
+void VariableDumper::dumpRight(const PDBSymbolTypePointer &Symbol) {
+  auto PointeeType = Symbol.getPointeeType();
+  assert(PointeeType);
+  if (!PointeeType)
+    return;
+  if (isa<PDBSymbolTypeFunctionSig>(PointeeType.get()) ||
+      isa<PDBSymbolTypeArray>(PointeeType.get())) {
+    Printer << ")";
+  }
+  PointeeType->dumpRight(*this);
 }
 
 void VariableDumper::dump(const PDBSymbolTypeTypedef &Symbol) {
   WithColor(Printer, PDB_ColorItem::Keyword).get() << "typedef ";
   WithColor(Printer, PDB_ColorItem::Type).get() << Symbol.getName();
 }
 
 void VariableDumper::dump(const PDBSymbolTypeUDT &Symbol) {
   WithColor(Printer, PDB_ColorItem::Type).get() << Symbol.getName();
 }
 
 void VariableDumper::dumpSymbolTypeAndName(const PDBSymbol &Type,
                                            StringRef Name) {
-  if (auto *ArrayType = Type.cast<PDBSymbolTypeArray>()) {
-    std::string IndexSpec;
-    raw_string_ostream IndexStream(IndexSpec);
-    std::unique_ptr<PDBSymbol> ElementType = ArrayType->getElementType();
-    while (auto NestedArray = ElementType->cast<PDBSymbolTypeArray>()) {
-      IndexStream << "[";
-      IndexStream << NestedArray->getCount();
-      IndexStream << "]";
-      ElementType = NestedArray->getElementType();
-    }
-    IndexStream << "[" << ArrayType->getCount() << "]";
-    ElementType->dump(*this);
-    WithColor(Printer, PDB_ColorItem::Identifier).get() << " " << Name;
-    Printer << IndexStream.str();
-  } else {
-    if (!tryDumpFunctionPointer(Type, Name)) {
-      Type.dump(*this);
-      WithColor(Printer, PDB_ColorItem::Identifier).get() << " " << Name;
-    }
-  }
-}
-
-bool VariableDumper::tryDumpFunctionPointer(const PDBSymbol &Type,
-                                            StringRef Name) {
-  // Function pointers come across as pointers to function signatures.  But the
-  // signature carries no name, so we have to handle this case separately.
-  if (auto *PointerType = Type.cast<PDBSymbolTypePointer>()) {
-    auto PointeeType = PointerType->getPointeeType();
-    if (auto *FunctionSig = PointeeType->cast<PDBSymbolTypeFunctionSig>()) {
-      FunctionDumper Dumper(Printer);
-      FunctionDumper::PointerType PT = FunctionDumper::PointerType::Pointer;
-      if (PointerType->isReference())
-        PT = FunctionDumper::PointerType::Reference;
-      std::string NameStr(Name.begin(), Name.end());
-      Dumper.start(*FunctionSig, NameStr.c_str(), PT);
-      return true;
-    }
-  }
-  return false;
+  Type.dumpRight(*this);
 }