[pdb] Change type visitor pattern to be dynamic.

This allows better catching of compiler errors since we can use
the override keyword to verify that methods are actually
overridden.

Also in this patch I've changed from storing a boolean Error
code everywhere to returning an llvm::Error, to propagate richer
error information up the call stack.

Reviewed By: ruiu, rnk
Differential Revision: http://reviews.llvm.org/D21410

llvm-svn: 272926
diff --git a/llvm/lib/DebugInfo/CodeView/CMakeLists.txt b/llvm/lib/DebugInfo/CodeView/CMakeLists.txt
index 4393f9c..47297a9 100644
--- a/llvm/lib/DebugInfo/CodeView/CMakeLists.txt
+++ b/llvm/lib/DebugInfo/CodeView/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_llvm_library(LLVMDebugInfoCodeView
   ByteStream.cpp
   CodeViewError.cpp
+  CVTypeVisitor.cpp
   EnumTables.cpp
   FieldListRecordBuilder.cpp
   Line.cpp
diff --git a/llvm/lib/DebugInfo/CodeView/CVTypeVisitor.cpp b/llvm/lib/DebugInfo/CodeView/CVTypeVisitor.cpp
new file mode 100644
index 0000000..a22d963
--- /dev/null
+++ b/llvm/lib/DebugInfo/CodeView/CVTypeVisitor.cpp
@@ -0,0 +1,119 @@
+//===- CVTypeVisitor.cpp ----------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/DebugInfo/CodeView/CVTypeVisitor.h"
+
+using namespace llvm;
+using namespace llvm::codeview;
+
+template <typename T>
+static Error takeObject(ArrayRef<uint8_t> &Data, const T *&Res) {
+  if (Data.size() < sizeof(*Res))
+    return llvm::make_error<CodeViewError>(cv_error_code::insufficient_buffer);
+  Res = reinterpret_cast<const T *>(Data.data());
+  Data = Data.drop_front(sizeof(*Res));
+  return Error::success();
+}
+
+CVTypeVisitor::CVTypeVisitor(TypeVisitorCallbacks &Callbacks)
+    : Callbacks(Callbacks) {}
+
+Error CVTypeVisitor::visitTypeRecord(const CVRecord<TypeLeafKind> &Record) {
+  ArrayRef<uint8_t> LeafData = Record.Data;
+  if (auto EC = Callbacks.visitTypeBegin(Record))
+    return EC;
+  switch (Record.Type) {
+  default:
+    if (auto EC = Callbacks.visitUnknownType(Record))
+      return EC;
+    break;
+  case LF_FIELDLIST:
+    if (auto EC = visitFieldList(Record))
+      return EC;
+    break;
+#define TYPE_RECORD(EnumName, EnumVal, Name)                                   \
+  case EnumName: {                                                             \
+    TypeRecordKind RK = static_cast<TypeRecordKind>(EnumName);                 \
+    auto Result = Name##Record::deserialize(RK, LeafData);                     \
+    if (Result.getError())                                                     \
+      return llvm::make_error<CodeViewError>(cv_error_code::corrupt_record);   \
+    if (auto EC = Callbacks.visit##Name(*Result))                              \
+      return EC;                                                               \
+    break;                                                                     \
+  }
+#define TYPE_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)                  \
+  TYPE_RECORD(EnumVal, EnumVal, AliasName)
+#define MEMBER_RECORD(EnumName, EnumVal, Name)
+#include "llvm/DebugInfo/CodeView/TypeRecords.def"
+  }
+  if (auto EC = Callbacks.visitTypeEnd(Record))
+    return EC;
+  return Error::success();
+}
+
+/// Visits the type records in Data. Sets the error flag on parse failures.
+Error CVTypeVisitor::visitTypeStream(const CVTypeArray &Types) {
+  for (const auto &I : Types) {
+    if (auto EC = visitTypeRecord(I))
+      return EC;
+  }
+  return Error::success();
+}
+
+Error CVTypeVisitor::skipPadding(ArrayRef<uint8_t> &Data) {
+  if (Data.empty())
+    return Error::success();
+  uint8_t Leaf = Data.front();
+  if (Leaf < LF_PAD0)
+    return Error::success();
+  // Leaf is greater than 0xf0. We should advance by the number of bytes in
+  // the low 4 bits.
+  unsigned BytesToAdvance = Leaf & 0x0F;
+  if (Data.size() < BytesToAdvance) {
+    return llvm::make_error<CodeViewError>(cv_error_code::corrupt_record,
+                                           "Invalid padding bytes!");
+  }
+  Data = Data.drop_front(BytesToAdvance);
+  return Error::success();
+}
+
+/// Visits individual member records of a field list record. Member records do
+/// not describe their own length, and need special handling.
+Error CVTypeVisitor::visitFieldList(const CVRecord<TypeLeafKind> &Record) {
+  ArrayRef<uint8_t> RecordData = Record.Data;
+  while (!RecordData.empty()) {
+    const ulittle16_t *LeafPtr;
+    if (auto EC = takeObject(RecordData, LeafPtr))
+      return EC;
+    TypeLeafKind Leaf = TypeLeafKind(unsigned(*LeafPtr));
+    switch (Leaf) {
+    default:
+      // Field list records do not describe their own length, so we cannot
+      // continue parsing past an unknown member type.
+      if (auto EC = Callbacks.visitUnknownMember(Record))
+        return llvm::make_error<CodeViewError>(cv_error_code::corrupt_record);
+#define MEMBER_RECORD(EnumName, EnumVal, Name)                                 \
+  case EnumName: {                                                             \
+    TypeRecordKind RK = static_cast<TypeRecordKind>(EnumName);                 \
+    auto Result = Name##Record::deserialize(RK, RecordData);                   \
+    if (Result.getError())                                                     \
+      return llvm::make_error<CodeViewError>(cv_error_code::corrupt_record);   \
+    if (auto EC = Callbacks.visit##Name(*Result))                              \
+      return EC;                                                               \
+    break;                                                                     \
+  }
+#define MEMBER_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)                \
+  MEMBER_RECORD(EnumVal, EnumVal, AliasName)
+#include "llvm/DebugInfo/CodeView/TypeRecords.def"
+    }
+    if (auto EC = skipPadding(RecordData))
+      return EC;
+  }
+  return Error::success();
+}
diff --git a/llvm/lib/DebugInfo/CodeView/TypeDumper.cpp b/llvm/lib/DebugInfo/CodeView/TypeDumper.cpp
index b181ffb..0653d81 100644
--- a/llvm/lib/DebugInfo/CodeView/TypeDumper.cpp
+++ b/llvm/lib/DebugInfo/CodeView/TypeDumper.cpp
@@ -189,55 +189,6 @@
 
 #undef ENUM_ENTRY
 
-
-namespace {
-
-/// Use this private dumper implementation to keep implementation details about
-/// the visitor out of TypeDumper.h.
-class CVTypeDumperImpl : public CVTypeVisitor<CVTypeDumperImpl> {
-public:
-  CVTypeDumperImpl(CVTypeDumper &CVTD, ScopedPrinter &W, bool PrintRecordBytes)
-      : CVTD(CVTD), W(W), PrintRecordBytes(PrintRecordBytes) {}
-
-  /// CVTypeVisitor overrides.
-#define TYPE_RECORD(EnumName, EnumVal, Name)                                   \
-  void visit##Name(Name##Record &Record);
-#define TYPE_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
-#define MEMBER_RECORD(EnumName, EnumVal, Name)                                 \
-  void visit##Name(Name##Record &Record);
-#define MEMBER_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
-#include "llvm/DebugInfo/CodeView/TypeRecords.def"
-
-  void visitUnknownMember(TypeLeafKind Leaf);
-  void visitUnknownType(const CVRecord<TypeLeafKind> &Record);
-
-  void visitTypeBegin(const CVRecord<TypeLeafKind> &Record);
-  void visitTypeEnd(const CVRecord<TypeLeafKind> &Record);
-
-  void printMemberAttributes(MemberAttributes Attrs);
-  void printMemberAttributes(MemberAccess Access, MethodKind Kind,
-                             MethodOptions Options);
-
-private:
-  /// Forwards to the dumper, which holds the persistent state from visitation.
-  StringRef getTypeName(TypeIndex TI) {
-    return CVTD.getTypeName(TI);
-  }
-
-  void printTypeIndex(StringRef FieldName, TypeIndex TI) {
-    CVTD.printTypeIndex(FieldName, TI);
-  }
-
-  CVTypeDumper &CVTD;
-  ScopedPrinter &W;
-  bool PrintRecordBytes = false;
-
-  /// Name of the current type. Only valid before visitTypeEnd.
-  StringRef Name;
-};
-
-} // end anonymous namespace
-
 static StringRef getLeafTypeName(TypeLeafKind LT) {
   switch (LT) {
 #define TYPE_RECORD(ename, value, name)                                        \
@@ -250,39 +201,44 @@
   return "UnknownLeaf";
 }
 
-void CVTypeDumperImpl::visitTypeBegin(const CVRecord<TypeLeafKind> &Rec) {
+Error CVTypeDumper::visitTypeBegin(const CVRecord<TypeLeafKind> &Record) {
   // Reset Name to the empty string. If the visitor sets it, we know it.
   Name = "";
 
-  W.startLine() << getLeafTypeName(Rec.Type) << " ("
-                << HexNumber(CVTD.getNextTypeIndex()) << ") {\n";
-  W.indent();
-  W.printEnum("TypeLeafKind", unsigned(Rec.Type), makeArrayRef(LeafTypeNames));
+  W->startLine() << getLeafTypeName(Record.Type) << " ("
+                 << HexNumber(getNextTypeIndex()) << ") {\n";
+  W->indent();
+  W->printEnum("TypeLeafKind", unsigned(Record.Type),
+               makeArrayRef(LeafTypeNames));
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitTypeEnd(const CVRecord<TypeLeafKind> &Rec) {
+Error CVTypeDumper::visitTypeEnd(const CVRecord<TypeLeafKind> &Record) {
   // Always record some name for every type, even if Name is empty. CVUDTNames
   // is indexed by type index, and must have one entry for every type.
-  CVTD.recordType(Name);
-  if (PrintRecordBytes)
-    W.printBinaryBlock("LeafData", getBytesAsCharacters(Rec.Data));
+  recordType(Name);
 
-  W.unindent();
-  W.startLine() << "}\n";
+  if (PrintRecordBytes)
+    W->printBinaryBlock("LeafData", getBytesAsCharacters(Record.Data));
+
+  W->unindent();
+  W->startLine() << "}\n";
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitStringId(StringIdRecord &String) {
+Error CVTypeDumper::visitStringId(StringIdRecord &String) {
   printTypeIndex("Id", String.getId());
-  W.printString("StringData", String.getString());
+  W->printString("StringData", String.getString());
   // Put this in CVUDTNames so it gets printed with LF_UDT_SRC_LINE.
   Name = String.getString();
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitArgList(ArgListRecord &Args) {
+Error CVTypeDumper::visitArgList(ArgListRecord &Args) {
   auto Indices = Args.getIndices();
   uint32_t Size = Indices.size();
-  W.printNumber("NumArgs", Size);
-  ListScope Arguments(W, "Arguments");
+  W->printNumber("NumArgs", Size);
+  ListScope Arguments(*W, "Arguments");
   SmallString<256> TypeName("(");
   for (uint32_t I = 0; I < Size; ++I) {
     printTypeIndex("ArgType", Indices[I]);
@@ -292,77 +248,84 @@
       TypeName.append(", ");
   }
   TypeName.push_back(')');
-  Name = CVTD.saveName(TypeName);
+  Name = saveName(TypeName);
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitClass(ClassRecord &Class) {
+Error CVTypeDumper::visitClass(ClassRecord &Class) {
   uint16_t Props = static_cast<uint16_t>(Class.getOptions());
-  W.printNumber("MemberCount", Class.getMemberCount());
-  W.printFlags("Properties", Props, makeArrayRef(ClassOptionNames));
+  W->printNumber("MemberCount", Class.getMemberCount());
+  W->printFlags("Properties", Props, makeArrayRef(ClassOptionNames));
   printTypeIndex("FieldList", Class.getFieldList());
   printTypeIndex("DerivedFrom", Class.getDerivationList());
   printTypeIndex("VShape", Class.getVTableShape());
-  W.printNumber("SizeOf", Class.getSize());
-  W.printString("Name", Class.getName());
+  W->printNumber("SizeOf", Class.getSize());
+  W->printString("Name", Class.getName());
   if (Props & uint16_t(ClassOptions::HasUniqueName))
-    W.printString("LinkageName", Class.getUniqueName());
+    W->printString("LinkageName", Class.getUniqueName());
   Name = Class.getName();
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitUnion(UnionRecord &Union) {
+Error CVTypeDumper::visitUnion(UnionRecord &Union) {
   uint16_t Props = static_cast<uint16_t>(Union.getOptions());
-  W.printNumber("MemberCount", Union.getMemberCount());
-  W.printFlags("Properties", Props, makeArrayRef(ClassOptionNames));
+  W->printNumber("MemberCount", Union.getMemberCount());
+  W->printFlags("Properties", Props, makeArrayRef(ClassOptionNames));
   printTypeIndex("FieldList", Union.getFieldList());
-  W.printNumber("SizeOf", Union.getSize());
-  W.printString("Name", Union.getName());
+  W->printNumber("SizeOf", Union.getSize());
+  W->printString("Name", Union.getName());
   if (Props & uint16_t(ClassOptions::HasUniqueName))
-    W.printString("LinkageName", Union.getUniqueName());
+    W->printString("LinkageName", Union.getUniqueName());
   Name = Union.getName();
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitEnum(EnumRecord &Enum) {
-  W.printNumber("NumEnumerators", Enum.getMemberCount());
-  W.printFlags("Properties", uint16_t(Enum.getOptions()),
-               makeArrayRef(ClassOptionNames));
+Error CVTypeDumper::visitEnum(EnumRecord &Enum) {
+  W->printNumber("NumEnumerators", Enum.getMemberCount());
+  W->printFlags("Properties", uint16_t(Enum.getOptions()),
+                makeArrayRef(ClassOptionNames));
   printTypeIndex("UnderlyingType", Enum.getUnderlyingType());
   printTypeIndex("FieldListType", Enum.getFieldList());
-  W.printString("Name", Enum.getName());
+  W->printString("Name", Enum.getName());
   Name = Enum.getName();
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitArray(ArrayRecord &AT) {
+Error CVTypeDumper::visitArray(ArrayRecord &AT) {
   printTypeIndex("ElementType", AT.getElementType());
   printTypeIndex("IndexType", AT.getIndexType());
-  W.printNumber("SizeOf", AT.getSize());
-  W.printString("Name", AT.getName());
+  W->printNumber("SizeOf", AT.getSize());
+  W->printString("Name", AT.getName());
   Name = AT.getName();
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitVFTable(VFTableRecord &VFT) {
+Error CVTypeDumper::visitVFTable(VFTableRecord &VFT) {
   printTypeIndex("CompleteClass", VFT.getCompleteClass());
   printTypeIndex("OverriddenVFTable", VFT.getOverriddenVTable());
-  W.printHex("VFPtrOffset", VFT.getVFPtrOffset());
-  W.printString("VFTableName", VFT.getName());
+  W->printHex("VFPtrOffset", VFT.getVFPtrOffset());
+  W->printString("VFTableName", VFT.getName());
   for (auto N : VFT.getMethodNames())
-    W.printString("MethodName", N);
+    W->printString("MethodName", N);
   Name = VFT.getName();
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitMemberFuncId(MemberFuncIdRecord &Id) {
+Error CVTypeDumper::visitMemberFuncId(MemberFuncIdRecord &Id) {
   printTypeIndex("ClassType", Id.getClassType());
   printTypeIndex("FunctionType", Id.getFunctionType());
-  W.printString("Name", Id.getName());
+  W->printString("Name", Id.getName());
   Name = Id.getName();
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitProcedure(ProcedureRecord &Proc) {
+Error CVTypeDumper::visitProcedure(ProcedureRecord &Proc) {
   printTypeIndex("ReturnType", Proc.getReturnType());
-  W.printEnum("CallingConvention", uint8_t(Proc.getCallConv()),
-              makeArrayRef(CallingConventions));
-  W.printFlags("FunctionOptions", uint8_t(Proc.getOptions()),
-               makeArrayRef(FunctionOptionEnum));
-  W.printNumber("NumParameters", Proc.getParameterCount());
+  W->printEnum("CallingConvention", uint8_t(Proc.getCallConv()),
+               makeArrayRef(CallingConventions));
+  W->printFlags("FunctionOptions", uint8_t(Proc.getOptions()),
+                makeArrayRef(FunctionOptionEnum));
+  W->printNumber("NumParameters", Proc.getParameterCount());
   printTypeIndex("ArgListType", Proc.getArgumentList());
 
   StringRef ReturnTypeName = getTypeName(Proc.getReturnType());
@@ -370,20 +333,21 @@
   SmallString<256> TypeName(ReturnTypeName);
   TypeName.push_back(' ');
   TypeName.append(ArgListTypeName);
-  Name = CVTD.saveName(TypeName);
+  Name = saveName(TypeName);
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitMemberFunction(MemberFunctionRecord &MF) {
+Error CVTypeDumper::visitMemberFunction(MemberFunctionRecord &MF) {
   printTypeIndex("ReturnType", MF.getReturnType());
   printTypeIndex("ClassType", MF.getClassType());
   printTypeIndex("ThisType", MF.getThisType());
-  W.printEnum("CallingConvention", uint8_t(MF.getCallConv()),
-              makeArrayRef(CallingConventions));
-  W.printFlags("FunctionOptions", uint8_t(MF.getOptions()),
-               makeArrayRef(FunctionOptionEnum));
-  W.printNumber("NumParameters", MF.getParameterCount());
+  W->printEnum("CallingConvention", uint8_t(MF.getCallConv()),
+               makeArrayRef(CallingConventions));
+  W->printFlags("FunctionOptions", uint8_t(MF.getOptions()),
+                makeArrayRef(FunctionOptionEnum));
+  W->printNumber("NumParameters", MF.getParameterCount());
   printTypeIndex("ArgListType", MF.getArgumentList());
-  W.printNumber("ThisAdjustment", MF.getThisPointerAdjustment());
+  W->printNumber("ThisAdjustment", MF.getThisPointerAdjustment());
 
   StringRef ReturnTypeName = getTypeName(MF.getReturnType());
   StringRef ClassTypeName = getTypeName(MF.getClassType());
@@ -393,52 +357,56 @@
   TypeName.append(ClassTypeName);
   TypeName.append("::");
   TypeName.append(ArgListTypeName);
-  Name = CVTD.saveName(TypeName);
+  Name = saveName(TypeName);
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitMethodOverloadList(
+Error CVTypeDumper::visitMethodOverloadList(
     MethodOverloadListRecord &MethodList) {
   for (auto &M : MethodList.getMethods()) {
-    ListScope S(W, "Method");
+    ListScope S(*W, "Method");
     printMemberAttributes(M.getAccess(), M.getKind(), M.getOptions());
     printTypeIndex("Type", M.getType());
     if (M.isIntroducingVirtual())
-      W.printHex("VFTableOffset", M.getVFTableOffset());
+      W->printHex("VFTableOffset", M.getVFTableOffset());
   }
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitFuncId(FuncIdRecord &Func) {
+Error CVTypeDumper::visitFuncId(FuncIdRecord &Func) {
   printTypeIndex("ParentScope", Func.getParentScope());
   printTypeIndex("FunctionType", Func.getFunctionType());
-  W.printString("Name", Func.getName());
+  W->printString("Name", Func.getName());
   Name = Func.getName();
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitTypeServer2(TypeServer2Record &TS) {
-  W.printBinary("Signature", TS.getGuid());
-  W.printNumber("Age", TS.getAge());
-  W.printString("Name", TS.getName());
+Error CVTypeDumper::visitTypeServer2(TypeServer2Record &TS) {
+  W->printBinary("Signature", TS.getGuid());
+  W->printNumber("Age", TS.getAge());
+  W->printString("Name", TS.getName());
   Name = TS.getName();
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitPointer(PointerRecord &Ptr) {
+Error CVTypeDumper::visitPointer(PointerRecord &Ptr) {
   printTypeIndex("PointeeType", Ptr.getReferentType());
-  W.printHex("PointerAttributes", uint32_t(Ptr.getOptions()));
-  W.printEnum("PtrType", unsigned(Ptr.getPointerKind()),
-              makeArrayRef(PtrKindNames));
-  W.printEnum("PtrMode", unsigned(Ptr.getMode()), makeArrayRef(PtrModeNames));
+  W->printHex("PointerAttributes", uint32_t(Ptr.getOptions()));
+  W->printEnum("PtrType", unsigned(Ptr.getPointerKind()),
+               makeArrayRef(PtrKindNames));
+  W->printEnum("PtrMode", unsigned(Ptr.getMode()), makeArrayRef(PtrModeNames));
 
-  W.printNumber("IsFlat", Ptr.isFlat());
-  W.printNumber("IsConst", Ptr.isConst());
-  W.printNumber("IsVolatile", Ptr.isVolatile());
-  W.printNumber("IsUnaligned", Ptr.isUnaligned());
+  W->printNumber("IsFlat", Ptr.isFlat());
+  W->printNumber("IsConst", Ptr.isConst());
+  W->printNumber("IsVolatile", Ptr.isVolatile());
+  W->printNumber("IsUnaligned", Ptr.isUnaligned());
 
   if (Ptr.isPointerToMember()) {
     const MemberPointerInfo &MI = Ptr.getMemberInfo();
 
     printTypeIndex("ClassType", MI.getContainingType());
-    W.printEnum("Representation", uint16_t(MI.getRepresentation()),
-                makeArrayRef(PtrMemberRepNames));
+    W->printEnum("Representation", uint16_t(MI.getRepresentation()),
+                 makeArrayRef(PtrMemberRepNames));
 
     StringRef PointeeName = getTypeName(Ptr.getReferentType());
     StringRef ClassName = getTypeName(MI.getContainingType());
@@ -446,7 +414,7 @@
     TypeName.push_back(' ');
     TypeName.append(ClassName);
     TypeName.append("::*");
-    Name = CVTD.saveName(TypeName);
+    Name = saveName(TypeName);
   } else {
     SmallString<256> TypeName;
     if (Ptr.isConst())
@@ -466,14 +434,15 @@
       TypeName.append("*");
 
     if (!TypeName.empty())
-      Name = CVTD.saveName(TypeName);
+      Name = saveName(TypeName);
   }
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitModifier(ModifierRecord &Mod) {
+Error CVTypeDumper::visitModifier(ModifierRecord &Mod) {
   uint16_t Mods = static_cast<uint16_t>(Mod.getModifiers());
   printTypeIndex("ModifiedType", Mod.getModifiedType());
-  W.printFlags("Modifiers", Mods, makeArrayRef(TypeModifierNames));
+  W->printFlags("Modifiers", Mods, makeArrayRef(TypeModifierNames));
 
   StringRef ModifiedName = getTypeName(Mod.getModifiedType());
   SmallString<256> TypeName;
@@ -484,146 +453,162 @@
   if (Mods & uint16_t(ModifierOptions::Unaligned))
     TypeName.append("__unaligned ");
   TypeName.append(ModifiedName);
-  Name = CVTD.saveName(TypeName);
+  Name = saveName(TypeName);
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitBitField(BitFieldRecord &BitField) {
+Error CVTypeDumper::visitBitField(BitFieldRecord &BitField) {
   printTypeIndex("Type", BitField.getType());
-  W.printNumber("BitSize", BitField.getBitSize());
-  W.printNumber("BitOffset", BitField.getBitOffset());
+  W->printNumber("BitSize", BitField.getBitSize());
+  W->printNumber("BitOffset", BitField.getBitOffset());
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitVFTableShape(VFTableShapeRecord &Shape) {
-  W.printNumber("VFEntryCount", Shape.getEntryCount());
+Error CVTypeDumper::visitVFTableShape(VFTableShapeRecord &Shape) {
+  W->printNumber("VFEntryCount", Shape.getEntryCount());
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitUdtSourceLine(UdtSourceLineRecord &Line) {
+Error CVTypeDumper::visitUdtSourceLine(UdtSourceLineRecord &Line) {
   printTypeIndex("UDT", Line.getUDT());
   printTypeIndex("SourceFile", Line.getSourceFile());
-  W.printNumber("LineNumber", Line.getLineNumber());
+  W->printNumber("LineNumber", Line.getLineNumber());
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitUdtModSourceLine(UdtModSourceLineRecord &Line) {
+Error CVTypeDumper::visitUdtModSourceLine(UdtModSourceLineRecord &Line) {
   printTypeIndex("UDT", Line.getUDT());
   printTypeIndex("SourceFile", Line.getSourceFile());
-  W.printNumber("LineNumber", Line.getLineNumber());
-  W.printNumber("Module", Line.getModule());
+  W->printNumber("LineNumber", Line.getLineNumber());
+  W->printNumber("Module", Line.getModule());
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitBuildInfo(BuildInfoRecord &Args) {
-  W.printNumber("NumArgs", static_cast<uint32_t>(Args.getArgs().size()));
+Error CVTypeDumper::visitBuildInfo(BuildInfoRecord &Args) {
+  W->printNumber("NumArgs", static_cast<uint32_t>(Args.getArgs().size()));
 
-  ListScope Arguments(W, "Arguments");
+  ListScope Arguments(*W, "Arguments");
   for (auto Arg : Args.getArgs()) {
     printTypeIndex("ArgType", Arg);
   }
+  return Error::success();
 }
 
-void CVTypeDumperImpl::printMemberAttributes(MemberAttributes Attrs) {
+void CVTypeDumper::printMemberAttributes(MemberAttributes Attrs) {
   return printMemberAttributes(Attrs.getAccess(), Attrs.getMethodKind(),
                                Attrs.getFlags());
 }
 
-void CVTypeDumperImpl::printMemberAttributes(MemberAccess Access,
-                                             MethodKind Kind,
-                                             MethodOptions Options) {
-  W.printEnum("AccessSpecifier", uint8_t(Access),
-              makeArrayRef(MemberAccessNames));
+void CVTypeDumper::printMemberAttributes(MemberAccess Access, MethodKind Kind,
+                                         MethodOptions Options) {
+  W->printEnum("AccessSpecifier", uint8_t(Access),
+               makeArrayRef(MemberAccessNames));
   // Data members will be vanilla. Don't try to print a method kind for them.
   if (Kind != MethodKind::Vanilla)
-    W.printEnum("MethodKind", unsigned(Kind), makeArrayRef(MemberKindNames));
+    W->printEnum("MethodKind", unsigned(Kind), makeArrayRef(MemberKindNames));
   if (Options != MethodOptions::None) {
-    W.printFlags("MethodOptions", unsigned(Options),
-                 makeArrayRef(MethodOptionNames));
+    W->printFlags("MethodOptions", unsigned(Options),
+                  makeArrayRef(MethodOptionNames));
   }
 }
 
-void CVTypeDumperImpl::visitUnknownMember(TypeLeafKind Leaf) {
-  W.printHex("UnknownMember", unsigned(Leaf));
+Error CVTypeDumper::visitUnknownMember(const CVRecord<TypeLeafKind> &Record) {
+  W->printHex("UnknownMember", unsigned(Record.Type));
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitUnknownType(const CVRecord<TypeLeafKind> &Rec) {
-  DictScope S(W, "UnknownType");
-  W.printEnum("Kind", uint16_t(Rec.Type), makeArrayRef(LeafTypeNames));
-  W.printNumber("Length", uint32_t(Rec.Data.size()));
+Error CVTypeDumper::visitUnknownType(const CVRecord<TypeLeafKind> &Record) {
+  DictScope S(*W, "UnknownType");
+  W->printEnum("Kind", uint16_t(Record.Type), makeArrayRef(LeafTypeNames));
+  W->printNumber("Length", uint32_t(Record.Data.size()));
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitNestedType(NestedTypeRecord &Nested) {
-  DictScope S(W, "NestedType");
+Error CVTypeDumper::visitNestedType(NestedTypeRecord &Nested) {
+  DictScope S(*W, "NestedType");
   printTypeIndex("Type", Nested.getNestedType());
-  W.printString("Name", Nested.getName());
+  W->printString("Name", Nested.getName());
   Name = Nested.getName();
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitOneMethod(OneMethodRecord &Method) {
-  DictScope S(W, "OneMethod");
+Error CVTypeDumper::visitOneMethod(OneMethodRecord &Method) {
+  DictScope S(*W, "OneMethod");
   MethodKind K = Method.getKind();
   printMemberAttributes(Method.getAccess(), K, Method.getOptions());
   printTypeIndex("Type", Method.getType());
   // If virtual, then read the vftable offset.
   if (Method.isIntroducingVirtual())
-    W.printHex("VFTableOffset", Method.getVFTableOffset());
-  W.printString("Name", Method.getName());
+    W->printHex("VFTableOffset", Method.getVFTableOffset());
+  W->printString("Name", Method.getName());
   Name = Method.getName();
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitOverloadedMethod(OverloadedMethodRecord &Method) {
-  DictScope S(W, "OverloadedMethod");
-  W.printHex("MethodCount", Method.getNumOverloads());
+Error CVTypeDumper::visitOverloadedMethod(OverloadedMethodRecord &Method) {
+  DictScope S(*W, "OverloadedMethod");
+  W->printHex("MethodCount", Method.getNumOverloads());
   printTypeIndex("MethodListIndex", Method.getMethodList());
-  W.printString("Name", Method.getName());
+  W->printString("Name", Method.getName());
   Name = Method.getName();
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitDataMember(DataMemberRecord &Field) {
-  DictScope S(W, "DataMember");
+Error CVTypeDumper::visitDataMember(DataMemberRecord &Field) {
+  DictScope S(*W, "DataMember");
   printMemberAttributes(Field.getAccess(), MethodKind::Vanilla,
                         MethodOptions::None);
   printTypeIndex("Type", Field.getType());
-  W.printHex("FieldOffset", Field.getFieldOffset());
-  W.printString("Name", Field.getName());
+  W->printHex("FieldOffset", Field.getFieldOffset());
+  W->printString("Name", Field.getName());
   Name = Field.getName();
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitStaticDataMember(StaticDataMemberRecord &Field) {
-  DictScope S(W, "StaticDataMember");
+Error CVTypeDumper::visitStaticDataMember(StaticDataMemberRecord &Field) {
+  DictScope S(*W, "StaticDataMember");
   printMemberAttributes(Field.getAccess(), MethodKind::Vanilla,
                         MethodOptions::None);
   printTypeIndex("Type", Field.getType());
-  W.printString("Name", Field.getName());
+  W->printString("Name", Field.getName());
   Name = Field.getName();
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitVFPtr(VFPtrRecord &VFTable) {
-  DictScope S(W, "VFPtr");
+Error CVTypeDumper::visitVFPtr(VFPtrRecord &VFTable) {
+  DictScope S(*W, "VFPtr");
   printTypeIndex("Type", VFTable.getType());
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitEnumerator(EnumeratorRecord &Enum) {
-  DictScope S(W, "Enumerator");
+Error CVTypeDumper::visitEnumerator(EnumeratorRecord &Enum) {
+  DictScope S(*W, "Enumerator");
   printMemberAttributes(Enum.getAccess(), MethodKind::Vanilla,
                         MethodOptions::None);
-  W.printNumber("EnumValue", Enum.getValue());
-  W.printString("Name", Enum.getName());
+  W->printNumber("EnumValue", Enum.getValue());
+  W->printString("Name", Enum.getName());
   Name = Enum.getName();
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitBaseClass(BaseClassRecord &Base) {
-  DictScope S(W, "BaseClass");
+Error CVTypeDumper::visitBaseClass(BaseClassRecord &Base) {
+  DictScope S(*W, "BaseClass");
   printMemberAttributes(Base.getAccess(), MethodKind::Vanilla,
                         MethodOptions::None);
   printTypeIndex("BaseType", Base.getBaseType());
-  W.printHex("BaseOffset", Base.getBaseOffset());
+  W->printHex("BaseOffset", Base.getBaseOffset());
+  return Error::success();
 }
 
-void CVTypeDumperImpl::visitVirtualBaseClass(VirtualBaseClassRecord &Base) {
-  DictScope S(W, "VirtualBaseClass");
+Error CVTypeDumper::visitVirtualBaseClass(VirtualBaseClassRecord &Base) {
+  DictScope S(*W, "VirtualBaseClass");
   printMemberAttributes(Base.getAccess(), MethodKind::Vanilla,
                         MethodOptions::None);
   printTypeIndex("BaseType", Base.getBaseType());
   printTypeIndex("VBPtrType", Base.getVBPtrType());
-  W.printHex("VBPtrOffset", Base.getVBPtrOffset());
-  W.printHex("VBTableIndex", Base.getVTableIndex());
+  W->printHex("VBPtrOffset", Base.getVBPtrOffset());
+  W->printHex("VBTableIndex", Base.getVTableIndex());
+  return Error::success();
 }
 
 StringRef CVTypeDumper::getTypeName(TypeIndex TI) {
@@ -663,28 +648,29 @@
     W->printHex(FieldName, TI.getIndex());
 }
 
-bool CVTypeDumper::dump(const CVRecord<TypeLeafKind> &Record) {
+Error CVTypeDumper::dump(const CVRecord<TypeLeafKind> &Record) {
   assert(W && "printer should not be null");
-  CVTypeDumperImpl Dumper(*this, *W, PrintRecordBytes);
-  Dumper.visitTypeRecord(Record);
-  return !Dumper.hadError();
+  CVTypeVisitor Visitor(*this);
+
+  if (auto EC = Visitor.visitTypeRecord(Record))
+    return EC;
+  return Error::success();
 }
 
-bool CVTypeDumper::dump(const CVTypeArray &Types) {
+Error CVTypeDumper::dump(const CVTypeArray &Types) {
   assert(W && "printer should not be null");
-  CVTypeDumperImpl Dumper(*this, *W, PrintRecordBytes);
-  Dumper.visitTypeStream(Types);
-  return !Dumper.hadError();
+  CVTypeVisitor Visitor(*this);
+  if (auto EC = Visitor.visitTypeStream(Types))
+    return EC;
+  return Error::success();
 }
 
-bool CVTypeDumper::dump(ArrayRef<uint8_t> Data) {
+Error CVTypeDumper::dump(ArrayRef<uint8_t> Data) {
   ByteStream<> Stream(Data);
   CVTypeArray Types;
   StreamReader Reader(Stream);
-  if (auto EC = Reader.readArray(Types, Reader.getLength())) {
-    consumeError(std::move(EC));
-    return false;
-  }
+  if (auto EC = Reader.readArray(Types, Reader.getLength()))
+    return EC;
 
   return dump(Types);
 }
diff --git a/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp b/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp
index f71a31d..b8b035f 100644
--- a/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp
+++ b/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp
@@ -15,6 +15,8 @@
 #include "llvm/DebugInfo/CodeView/StreamRef.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
+#include "llvm/DebugInfo/CodeView/TypeVisitorCallbacks.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/ScopedPrinter.h"
 
 using namespace llvm;
@@ -49,32 +51,30 @@
 /// - If the type record already exists in the destination stream, discard it
 ///   and update the type index map to forward the source type index to the
 ///   existing destination type index.
-class TypeStreamMerger : public CVTypeVisitor<TypeStreamMerger> {
+class TypeStreamMerger : public TypeVisitorCallbacks {
 public:
   TypeStreamMerger(TypeTableBuilder &DestStream) : DestStream(DestStream) {
     assert(!hadError());
   }
 
-  /// CVTypeVisitor overrides.
+/// TypeVisitorCallbacks overrides.
 #define TYPE_RECORD(EnumName, EnumVal, Name)                                   \
-  void visit##Name(Name##Record &Record);
-#define TYPE_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
+  Error visit##Name(Name##Record &Record) override;
 #define MEMBER_RECORD(EnumName, EnumVal, Name)                                 \
-  void visit##Name(Name##Record &Record);
+  TYPE_RECORD(EnumName, EnumVal, Name)
+#define TYPE_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
 #define MEMBER_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
 #include "llvm/DebugInfo/CodeView/TypeRecords.def"
 
-  void visitUnknownType(const CVRecord<TypeLeafKind> &Record);
+  Error visitUnknownType(const CVRecord<TypeLeafKind> &Record) override;
 
-  void visitTypeBegin(const CVRecord<TypeLeafKind> &Record);
-  void visitTypeEnd(const CVRecord<TypeLeafKind> &Record);
-
-  void visitFieldList(TypeLeafKind Leaf, ArrayRef<uint8_t> FieldData);
+  Error visitTypeBegin(const CVRecord<TypeLeafKind> &Record) override;
+  Error visitTypeEnd(const CVRecord<TypeLeafKind> &Record) override;
 
   bool mergeStream(const CVTypeArray &Types);
 
 private:
-  bool hadError() { return FoundBadTypeIndex || CVTypeVisitor::hadError(); }
+  bool hadError() { return FoundBadTypeIndex; }
 
   bool FoundBadTypeIndex = false;
 
@@ -91,45 +91,46 @@
 
 } // end anonymous namespace
 
-void TypeStreamMerger::visitTypeBegin(const CVRecord<TypeLeafKind> &Rec) {
+Error TypeStreamMerger::visitTypeBegin(const CVRecord<TypeLeafKind> &Rec) {
   BeginIndexMapSize = IndexMap.size();
+  return Error::success();
 }
 
-void TypeStreamMerger::visitTypeEnd(const CVRecord<TypeLeafKind> &Rec) {
+Error TypeStreamMerger::visitTypeEnd(const CVRecord<TypeLeafKind> &Rec) {
   assert(IndexMap.size() == BeginIndexMapSize + 1);
-}
-
-void TypeStreamMerger::visitFieldList(TypeLeafKind Leaf,
-                                      ArrayRef<uint8_t> FieldData) {
-  CVTypeVisitor::visitFieldList(Leaf, FieldData);
-  IndexMap.push_back(DestStream.writeFieldList(FieldBuilder));
-  FieldBuilder.reset();
+  return Error::success();
 }
 
 #define TYPE_RECORD(EnumName, EnumVal, Name)                                   \
-  void TypeStreamMerger::visit##Name(Name##Record &Record) {                   \
+  Error TypeStreamMerger::visit##Name(Name##Record &Record) {                  \
     FoundBadTypeIndex |= !Record.remapTypeIndices(IndexMap);                   \
     IndexMap.push_back(DestStream.write##Name(Record));                        \
+    return Error::success();                                                   \
   }
 #define TYPE_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
 #define MEMBER_RECORD(EnumName, EnumVal, Name)                                 \
-  void TypeStreamMerger::visit##Name(Name##Record &Record) {                   \
+  Error TypeStreamMerger::visit##Name(Name##Record &Record) {                  \
     FoundBadTypeIndex |= !Record.remapTypeIndices(IndexMap);                   \
     FieldBuilder.write##Name(Record);                                          \
+    return Error::success();                                                   \
   }
 #define MEMBER_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
 #include "llvm/DebugInfo/CodeView/TypeRecords.def"
 
-void TypeStreamMerger::visitUnknownType(const CVRecord<TypeLeafKind> &Rec) {
+Error TypeStreamMerger::visitUnknownType(const CVRecord<TypeLeafKind> &Rec) {
   // We failed to translate a type. Translate this index as "not translated".
   IndexMap.push_back(
       TypeIndex(SimpleTypeKind::NotTranslated, SimpleTypeMode::Direct));
-  parseError();
+  return llvm::make_error<CodeViewError>(cv_error_code::corrupt_record);
 }
 
 bool TypeStreamMerger::mergeStream(const CVTypeArray &Types) {
   assert(IndexMap.empty());
-  visitTypeStream(Types);
+  CVTypeVisitor Visitor(*this);
+  if (auto EC = Visitor.visitTypeStream(Types)) {
+    consumeError(std::move(EC));
+    return false;
+  }
   IndexMap.clear();
   return !hadError();
 }
diff --git a/llvm/lib/DebugInfo/PDB/Raw/RawError.cpp b/llvm/lib/DebugInfo/PDB/Raw/RawError.cpp
index 07aebb7..eb169f7 100644
--- a/llvm/lib/DebugInfo/PDB/Raw/RawError.cpp
+++ b/llvm/lib/DebugInfo/PDB/Raw/RawError.cpp
@@ -32,6 +32,8 @@
       return "The specified block address is not valid.";
     case raw_error_code::not_writable:
       return "The PDB does not support writing.";
+    case raw_error_code::invalid_tpi_hash:
+      return "The Type record has an invalid hash value.";
     }
     llvm_unreachable("Unrecognized raw_error_code");
   }
diff --git a/llvm/lib/DebugInfo/PDB/Raw/TpiStream.cpp b/llvm/lib/DebugInfo/PDB/Raw/TpiStream.cpp
index 99e5037..46717f2 100644
--- a/llvm/lib/DebugInfo/PDB/Raw/TpiStream.cpp
+++ b/llvm/lib/DebugInfo/PDB/Raw/TpiStream.cpp
@@ -84,39 +84,44 @@
 }
 
 namespace {
-class TpiHashVerifier : public CVTypeVisitor<TpiHashVerifier> {
+class TpiHashVerifier : public TypeVisitorCallbacks {
 public:
   TpiHashVerifier(FixedStreamArray<support::ulittle32_t> &HashValues,
                   uint32_t NumHashBuckets)
       : HashValues(HashValues), NumHashBuckets(NumHashBuckets) {}
 
-  void visitUdtSourceLine(UdtSourceLineRecord &Rec) { verifySourceLine(Rec); }
-
-  void visitUdtModSourceLine(UdtModSourceLineRecord &Rec) {
-    verifySourceLine(Rec);
+  Error visitUdtSourceLine(UdtSourceLineRecord &Rec) override {
+    return verifySourceLine(Rec);
   }
 
-  void visitClass(ClassRecord &Rec) { verify(Rec); }
-  void visitEnum(EnumRecord &Rec) { verify(Rec); }
-  void visitInterface(ClassRecord &Rec) { verify(Rec); }
-  void visitStruct(ClassRecord &Rec) { verify(Rec); }
-  void visitUnion(UnionRecord &Rec) { verify(Rec); }
+  Error visitUdtModSourceLine(UdtModSourceLineRecord &Rec) override {
+    return verifySourceLine(Rec);
+  }
 
-  void visitTypeEnd(const CVRecord<TypeLeafKind> &Record) { ++Index; }
+  Error visitClass(ClassRecord &Rec) override { return verify(Rec); }
+  Error visitEnum(EnumRecord &Rec) override { return verify(Rec); }
+  Error visitUnion(UnionRecord &Rec) override { return verify(Rec); }
+
+  Error visitTypeEnd(const CVRecord<TypeLeafKind> &Record) override {
+    ++Index;
+    return Error::success();
+  }
 
 private:
-  template <typename T> void verify(T &Rec) {
+  template <typename T> Error verify(T &Rec) {
     uint32_t Hash = getTpiHash(Rec);
     if (Hash && Hash % NumHashBuckets != HashValues[Index])
-      parseError();
+      return make_error<RawError>(raw_error_code::invalid_tpi_hash);
+    return Error::success();
   }
 
-  template <typename T> void verifySourceLine(T &Rec) {
+  template <typename T> Error verifySourceLine(T &Rec) {
     char Buf[4];
     support::endian::write32le(Buf, Rec.getUDT().getIndex());
     uint32_t Hash = hashStringV1(StringRef(Buf, 4));
     if (Hash % NumHashBuckets != HashValues[Index])
-      parseError();
+      return make_error<RawError>(raw_error_code::invalid_tpi_hash);
+    return Error::success();
   }
 
   FixedStreamArray<support::ulittle32_t> HashValues;
@@ -129,11 +134,8 @@
 // Currently we only verify SRC_LINE records.
 Error TpiStream::verifyHashValues() {
   TpiHashVerifier Verifier(HashValues, Header->NumHashBuckets);
-  Verifier.visitTypeStream(TypeRecords);
-  if (Verifier.hadError())
-    return make_error<RawError>(raw_error_code::corrupt_file,
-                                "Corrupt TPI hash table.");
-  return Error::success();
+  CVTypeVisitor Visitor(Verifier);
+  return Visitor.visitTypeStream(TypeRecords);
 }
 
 Error TpiStream::reload() {