[CodeView] Simplify the use of visiting type records & streams.

There is often a lot of boilerplate code required to visit a type
record or type stream.  The #1 use case is that you have a sequence
of bytes that represent one or more records, and you want to
deserialize each one, switch on it, and call a callback with the
deserialized record that the user can examine.  Currently this
requires at least 6 lines of code:

  codeview::TypeVisitorCallbackPipeline Pipeline;
  Pipeline.addCallbackToPipeline(Deserializer);
  Pipeline.addCallbackToPipeline(MyCallbacks);

  codeview::CVTypeVisitor Visitor(Pipeline);
  consumeError(Visitor.visitTypeRecord(Record));

With this patch, it becomes one line of code:

  consumeError(codeview::visitTypeRecord(Record, MyCallbacks));

This is done by having the deserialization happen internally inside
of the visitTypeRecord function.  Since this is occasionally not
desirable, the function provides a 3rd parameter that can be used
to change this behavior.

Hopefully this can significantly reduce the barrier to entry
to using the visitation infrastructure.

Differential Revision: https://reviews.llvm.org/D33245

llvm-svn: 303271

include/llvm/DebugInfo/CodeView/CVTypeVisitor.h

@@ -28,7 +28,7 @@ public:
 
   Error visitTypeRecord(CVType &Record, TypeIndex Index);
   Error visitTypeRecord(CVType &Record);
-  Error visitMemberRecord(CVMemberRecord &Record);
+  Error visitMemberRecord(CVMemberRecord Record);
 
   /// Visits the type records in Data. Sets the error flag on parse failures.
   Error visitTypeStream(const CVTypeArray &Types);
@@ -47,6 +47,36 @@ private:
   TinyPtrVector<TypeServerHandler *> Handlers;
 };
 
+enum VisitorDataSource {
+  VDS_BytesPresent, // The record bytes are passed into the the visitation
+                    // function.  The algorithm should first deserialize them
+                    // before passing them on through the pipeline.
+  VDS_BytesExternal // The record bytes are not present, and it is the
+                    // responsibility of the visitor callback interface to
+                    // supply the bytes.
+};
+
+Error visitTypeRecord(CVType &Record, TypeIndex Index,
+                      TypeVisitorCallbacks &Callbacks,
+                      VisitorDataSource Source = VDS_BytesPresent,
+                      TypeServerHandler *TS = nullptr);
+Error visitTypeRecord(CVType &Record, TypeVisitorCallbacks &Callbacks,
+                      VisitorDataSource Source = VDS_BytesPresent,
+                      TypeServerHandler *TS = nullptr);
+
+Error visitMemberRecord(CVMemberRecord Record, TypeVisitorCallbacks &Callbacks,
+                        VisitorDataSource Source = VDS_BytesPresent);
+Error visitMemberRecord(TypeLeafKind Kind, ArrayRef<uint8_t> Record,
+                        TypeVisitorCallbacks &Callbacks);
+
+Error visitMemberRecordStream(ArrayRef<uint8_t> FieldList,
+                              TypeVisitorCallbacks &Callbacks);
+
+Error visitTypeStream(const CVTypeArray &Types, TypeVisitorCallbacks &Callbacks,
+                      TypeServerHandler *TS = nullptr);
+Error visitTypeStream(CVTypeRange Types, TypeVisitorCallbacks &Callbacks,
+                      TypeServerHandler *TS = nullptr);
+
 } // end namespace codeview
 } // end namespace llvm
 

include/llvm/DebugInfo/CodeView/RandomAccessTypeVisitor.h

@@ -11,13 +11,10 @@
 #define LLVM_DEBUGINFO_CODEVIEW_RANDOMACCESSTYPEVISITOR_H
 
 #include "llvm/ADT/TinyPtrVector.h"
-#include "llvm/DebugInfo/CodeView/CVTypeVisitor.h"
 #include "llvm/DebugInfo/CodeView/TypeDatabase.h"
 #include "llvm/DebugInfo/CodeView/TypeDatabaseVisitor.h"
-#include "llvm/DebugInfo/CodeView/TypeDeserializer.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
-#include "llvm/DebugInfo/CodeView/TypeVisitorCallbackPipeline.h"
 #include "llvm/Support/Error.h"
 
 namespace llvm {
@@ -73,18 +70,6 @@ private:
   /// The database visitor which adds new records to the database.
   TypeDatabaseVisitor DatabaseVisitor;
 
-  /// The deserializer which deserializes new records.
-  TypeDeserializer Deserializer;
-
-  /// The visitation callback pipeline to use.  By default this contains a
-  /// deserializer and a type database visitor.  But the callback specified
-  /// in the constructor is also added.
-  TypeVisitorCallbackPipeline Pipeline;
-
-  /// The visitor used to visit the internal pipeline for deserialization and
-  /// database maintenance.
-  CVTypeVisitor InternalVisitor;
-
   /// A vector mapping type indices to type offset.  For every record that has
   /// been visited, contains the absolute offset of that record in the record
   /// array.

include/llvm/DebugInfo/PDB/Native/TpiStream.h

@@ -51,6 +51,7 @@ public:
   HashTable &getHashAdjusters();
 
   codeview::CVTypeRange types(bool *HadError) const;
+  const codeview::CVTypeArray &typeArray() const { return TypeRecords; }
 
   Error commit();
 

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

@@ -501,7 +501,7 @@ void CodeViewDebug::emitTypeInformation() {
       Error E = Reader.readArray(Types, Reader.getLength());
       if (!E) {
         TypeVisitorCallbacks C;
-        E = CVTypeVisitor(C).visitTypeStream(Types);
+        E = codeview::visitTypeStream(Types, C);
       }
       if (E) {
         logAllUnhandledErrors(std::move(E), errs(), "error: ");

lib/DebugInfo/CodeView/CVTypeDumper.cpp

@@ -11,7 +11,6 @@
 #include "llvm/DebugInfo/CodeView/CVTypeVisitor.h"
 #include "llvm/DebugInfo/CodeView/TypeDatabase.h"
 #include "llvm/DebugInfo/CodeView/TypeDatabaseVisitor.h"
-#include "llvm/DebugInfo/CodeView/TypeDeserializer.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
 #include "llvm/DebugInfo/CodeView/TypeVisitorCallbackPipeline.h"
 #include "llvm/Support/BinaryByteStream.h"
@@ -21,38 +20,23 @@ using namespace llvm::codeview;
 
 Error CVTypeDumper::dump(const CVType &Record, TypeVisitorCallbacks &Dumper) {
   TypeDatabaseVisitor DBV(TypeDB);
-  TypeDeserializer Deserializer;
   TypeVisitorCallbackPipeline Pipeline;
-  Pipeline.addCallbackToPipeline(Deserializer);
   Pipeline.addCallbackToPipeline(DBV);
   Pipeline.addCallbackToPipeline(Dumper);
 
-  CVTypeVisitor Visitor(Pipeline);
-  if (Handler)
-    Visitor.addTypeServerHandler(*Handler);
-
   CVType RecordCopy = Record;
-  if (auto EC = Visitor.visitTypeRecord(RecordCopy))
-    return EC;
-  return Error::success();
+  return codeview::visitTypeRecord(RecordCopy, Pipeline, VDS_BytesPresent,
+                                   Handler);
 }
 
 Error CVTypeDumper::dump(const CVTypeArray &Types,
                          TypeVisitorCallbacks &Dumper) {
   TypeDatabaseVisitor DBV(TypeDB);
-  TypeDeserializer Deserializer;
   TypeVisitorCallbackPipeline Pipeline;
-  Pipeline.addCallbackToPipeline(Deserializer);
   Pipeline.addCallbackToPipeline(DBV);
   Pipeline.addCallbackToPipeline(Dumper);
 
-  CVTypeVisitor Visitor(Pipeline);
-  if (Handler)
-    Visitor.addTypeServerHandler(*Handler);
-
-  if (auto EC = Visitor.visitTypeStream(Types))
-    return EC;
-  return Error::success();
+  return codeview::visitTypeStream(Types, Pipeline, Handler);
 }
 
 Error CVTypeDumper::dump(ArrayRef<uint8_t> Data, TypeVisitorCallbacks &Dumper) {

lib/DebugInfo/CodeView/CVTypeVisitor.cpp

@@ -59,13 +59,8 @@ static Expected<TypeServer2Record> deserializeTypeServerRecord(CVType &Record) {
   };
 
   TypeServer2Record R(TypeRecordKind::TypeServer2);
-  TypeDeserializer Deserializer;
   StealTypeServerVisitor Thief(R);
-  TypeVisitorCallbackPipeline Pipeline;
-  Pipeline.addCallbackToPipeline(Deserializer);
-  Pipeline.addCallbackToPipeline(Thief);
-  CVTypeVisitor Visitor(Pipeline);
-  if (auto EC = Visitor.visitTypeRecord(Record))
+  if (auto EC = visitTypeRecord(Record, Thief))
     return std::move(EC);
 
   return R;
@@ -178,7 +173,7 @@ static Error visitMemberRecord(CVMemberRecord &Record,
   return Error::success();
 }
 
-Error CVTypeVisitor::visitMemberRecord(CVMemberRecord &Record) {
+Error CVTypeVisitor::visitMemberRecord(CVMemberRecord Record) {
   return ::visitMemberRecord(Record, Callbacks);
 }
 
@@ -224,3 +219,93 @@ Error CVTypeVisitor::visitFieldListMemberStream(ArrayRef<uint8_t> Data) {
   BinaryStreamReader SR(S);
   return visitFieldListMemberStream(SR);
 }
+
+namespace {
+struct FieldListVisitHelper {
+  FieldListVisitHelper(TypeVisitorCallbacks &Callbacks, ArrayRef<uint8_t> Data,
+                       VisitorDataSource Source)
+      : Stream(Data, llvm::support::little), Reader(Stream),
+        Deserializer(Reader),
+        Visitor((Source == VDS_BytesPresent) ? Pipeline : Callbacks) {
+    if (Source == VDS_BytesPresent) {
+      Pipeline.addCallbackToPipeline(Deserializer);
+      Pipeline.addCallbackToPipeline(Callbacks);
+    }
+  }
+
+  BinaryByteStream Stream;
+  BinaryStreamReader Reader;
+  FieldListDeserializer Deserializer;
+  TypeVisitorCallbackPipeline Pipeline;
+  CVTypeVisitor Visitor;
+};
+
+struct VisitHelper {
+  VisitHelper(TypeVisitorCallbacks &Callbacks, VisitorDataSource Source,
+              TypeServerHandler *TS)
+      : Visitor((Source == VDS_BytesPresent) ? Pipeline : Callbacks) {
+    if (TS)
+      Visitor.addTypeServerHandler(*TS);
+    if (Source == VDS_BytesPresent) {
+      Pipeline.addCallbackToPipeline(Deserializer);
+      Pipeline.addCallbackToPipeline(Callbacks);
+    }
+  }
+
+  TypeDeserializer Deserializer;
+  TypeVisitorCallbackPipeline Pipeline;
+  CVTypeVisitor Visitor;
+};
+}
+
+Error llvm::codeview::visitTypeRecord(CVType &Record, TypeIndex Index,
+                                      TypeVisitorCallbacks &Callbacks,
+                                      VisitorDataSource Source,
+                                      TypeServerHandler *TS) {
+  VisitHelper Helper(Callbacks, Source, TS);
+  return Helper.Visitor.visitTypeRecord(Record, Index);
+}
+
+Error llvm::codeview::visitTypeRecord(CVType &Record,
+                                      TypeVisitorCallbacks &Callbacks,
+                                      VisitorDataSource Source,
+                                      TypeServerHandler *TS) {
+  VisitHelper Helper(Callbacks, Source, TS);
+  return Helper.Visitor.visitTypeRecord(Record);
+}
+
+Error llvm::codeview::visitMemberRecordStream(ArrayRef<uint8_t> FieldList,
+                                              TypeVisitorCallbacks &Callbacks) {
+  CVTypeVisitor Visitor(Callbacks);
+  return Visitor.visitFieldListMemberStream(FieldList);
+}
+
+Error llvm::codeview::visitMemberRecord(CVMemberRecord Record,
+                                        TypeVisitorCallbacks &Callbacks,
+                                        VisitorDataSource Source) {
+  FieldListVisitHelper Helper(Callbacks, Record.Data, Source);
+  return Helper.Visitor.visitMemberRecord(Record);
+}
+
+Error llvm::codeview::visitMemberRecord(TypeLeafKind Kind,
+                                        ArrayRef<uint8_t> Record,
+                                        TypeVisitorCallbacks &Callbacks) {
+  CVMemberRecord R;
+  R.Data = Record;
+  R.Kind = Kind;
+  return visitMemberRecord(R, Callbacks, VDS_BytesPresent);
+}
+
+Error llvm::codeview::visitTypeStream(const CVTypeArray &Types,
+                                      TypeVisitorCallbacks &Callbacks,
+                                      TypeServerHandler *TS) {
+  VisitHelper Helper(Callbacks, VDS_BytesPresent, TS);
+  return Helper.Visitor.visitTypeStream(Types);
+}
+
+Error llvm::codeview::visitTypeStream(CVTypeRange Types,
+                                      TypeVisitorCallbacks &Callbacks,
+                                      TypeServerHandler *TS) {
+  VisitHelper Helper(Callbacks, VDS_BytesPresent, TS);
+  return Helper.Visitor.visitTypeStream(Types);
+}

lib/DebugInfo/CodeView/RandomAccessTypeVisitor.cpp

@@ -9,6 +9,7 @@
 
 #include "llvm/DebugInfo/CodeView/RandomAccessTypeVisitor.h"
 
+#include "llvm/DebugInfo/CodeView/CVTypeVisitor.h"
 #include "llvm/DebugInfo/CodeView/TypeDatabase.h"
 #include "llvm/DebugInfo/CodeView/TypeServerHandler.h"
 #include "llvm/DebugInfo/CodeView/TypeVisitorCallbacks.h"
@@ -20,9 +21,7 @@ RandomAccessTypeVisitor::RandomAccessTypeVisitor(
     const CVTypeArray &Types, uint32_t NumRecords,
     PartialOffsetArray PartialOffsets)
     : Database(NumRecords), Types(Types), DatabaseVisitor(Database),
-      InternalVisitor(Pipeline), PartialOffsets(PartialOffsets) {
-  Pipeline.addCallbackToPipeline(Deserializer);
-  Pipeline.addCallbackToPipeline(DatabaseVisitor);
+      PartialOffsets(PartialOffsets) {
 
   KnownOffsets.resize(Database.capacity());
 }
@@ -38,8 +37,7 @@ Error RandomAccessTypeVisitor::visitTypeIndex(TypeIndex TI,
 
   assert(Database.contains(TI));
   auto &Record = Database.getTypeRecord(TI);
-  CVTypeVisitor V(Callbacks);
-  return V.visitTypeRecord(Record, TI);
+  return codeview::visitTypeRecord(Record, TI, Callbacks);
 }
 
 Error RandomAccessTypeVisitor::visitRangeForType(TypeIndex TI) {
@@ -78,7 +76,7 @@ Error RandomAccessTypeVisitor::visitRange(TypeIndex Begin, uint32_t BeginOffset,
 
   while (Begin != End) {
     assert(!Database.contains(Begin));
-    if (auto EC = InternalVisitor.visitTypeRecord(*RI, Begin))
+    if (auto EC = codeview::visitTypeRecord(*RI, Begin, DatabaseVisitor))
       return EC;
     KnownOffsets[Begin.toArrayIndex()] = BeginOffset;
 

lib/DebugInfo/CodeView/TypeDumpVisitor.cpp

@@ -216,8 +216,7 @@ Error TypeDumpVisitor::visitMemberEnd(CVMemberRecord &Record) {
 
 Error TypeDumpVisitor::visitKnownRecord(CVType &CVR,
                                         FieldListRecord &FieldList) {
-  CVTypeVisitor Visitor(*this);
-  if (auto EC = Visitor.visitFieldListMemberStream(FieldList.Data))
+  if (auto EC = codeview::visitMemberRecordStream(FieldList.Data, *this))
     return EC;
 
   return Error::success();

lib/DebugInfo/CodeView/TypeStreamMerger.cpp

@@ -361,8 +361,7 @@ Error TypeStreamMerger::visitKnownRecord(CVType &, FieldListRecord &R) {
   // Visit the members inside the field list.
   HadUntranslatedMember = false;
   FieldListBuilder.begin();
-  CVTypeVisitor Visitor(*this);
-  if (auto EC = Visitor.visitFieldListMemberStream(R.Data))
+  if (auto EC = codeview::visitMemberRecordStream(R.Data, *this))
     return EC;
 
   // Write the record if we translated all field list members.
@@ -440,18 +439,9 @@ Error TypeStreamMerger::visitUnknownType(CVType &Rec) {
 
 Error TypeStreamMerger::mergeStream(const CVTypeArray &Types) {
   assert(IndexMap.empty());
-  TypeVisitorCallbackPipeline Pipeline;
   LastError = Error::success();
 
-  TypeDeserializer Deserializer;
-  Pipeline.addCallbackToPipeline(Deserializer);
-  Pipeline.addCallbackToPipeline(*this);
-
-  CVTypeVisitor Visitor(Pipeline);
-  if (Handler)
-    Visitor.addTypeServerHandler(*Handler);
-
-  if (auto EC = Visitor.visitTypeStream(Types))
+  if (auto EC = codeview::visitTypeStream(Types, *this, Handler))
     return EC;
 
   // If we found bad indices but no other errors, try doing another pass and see
@@ -466,7 +456,8 @@ Error TypeStreamMerger::mergeStream(const CVTypeArray &Types) {
     IsSecondPass = true;
     NumBadIndices = 0;
     CurIndex = TypeIndex(TypeIndex::FirstNonSimpleIndex);
-    if (auto EC = Visitor.visitTypeStream(Types))
+
+    if (auto EC = codeview::visitTypeStream(Types, *this, Handler))
       return EC;
 
     assert(NumBadIndices <= BadIndicesRemaining &&

lib/DebugInfo/PDB/Native/PDBTypeServerHandler.cpp

@@ -55,9 +55,8 @@ PDBTypeServerHandler::handleInternal(PDBFile &File,
   auto ExpectedTpi = File.getPDBTpiStream();
   if (!ExpectedTpi)
     return ExpectedTpi.takeError();
-  CVTypeVisitor Visitor(Callbacks);
 
-  if (auto EC = Visitor.visitTypeStream(ExpectedTpi->types(nullptr)))
+  if (auto EC = codeview::visitTypeStream(ExpectedTpi->typeArray(), Callbacks))
     return std::move(EC);
 
   return true;

tools/llvm-pdbdump/Analyze.cpp

@@ -76,26 +76,15 @@ Error AnalysisStyle::dump() {
 
   TypeDatabase TypeDB(Tpi->getNumTypeRecords());
   TypeDatabaseVisitor DBV(TypeDB);
-  TypeDeserializer Deserializer;
   TypeVisitorCallbackPipeline Pipeline;
   HashLookupVisitor Hasher(*Tpi);
-  // Deserialize the types
-  Pipeline.addCallbackToPipeline(Deserializer);
   // Add them to the database
   Pipeline.addCallbackToPipeline(DBV);
   // Store their hash values
   Pipeline.addCallbackToPipeline(Hasher);
 
-  CVTypeVisitor Visitor(Pipeline);
-
-  bool Error = false;
-  for (auto Item : Tpi->types(&Error)) {
-    if (auto EC = Visitor.visitTypeRecord(Item))
-      return EC;
-  }
-  if (Error)
-    return make_error<RawError>(raw_error_code::corrupt_file,
-                                "TPI stream contained corrupt record");
+  if (auto EC = codeview::visitTypeStream(Tpi->typeArray(), Pipeline))
+    return EC;
 
   auto &Adjusters = Tpi->getHashAdjusters();
   DenseSet<uint32_t> AdjusterSet;

tools/llvm-pdbdump/LLVMOutputStyle.cpp

@@ -178,11 +178,10 @@ public:
 private:
   Error dumpTypeRecord(StringRef Label, TypeDatabase &DB, TypeIndex Index) {
     CompactTypeDumpVisitor CTDV(DB, Index, &P);
-    CVTypeVisitor Visitor(CTDV);
     DictScope D(P, Label);
     if (DB.contains(Index)) {
       CVType &Type = DB.getTypeRecord(Index);
-      if (auto EC = Visitor.visitTypeRecord(Type))
+      if (auto EC = codeview::visitTypeRecord(Type, CTDV))
         return EC;
     } else {
       P.printString(
@@ -629,7 +628,6 @@ Error LLVMOutputStyle::dumpTpiStream(uint32_t StreamIdx) {
 
   std::vector<std::unique_ptr<TypeVisitorCallbacks>> Visitors;
 
-  Visitors.push_back(make_unique<TypeDeserializer>());
   if (!StreamDB.hasValue()) {
     StreamDB.emplace(Tpi->getNumTypeRecords());
     Visitors.push_back(make_unique<TypeDatabaseVisitor>(*StreamDB));
@@ -659,8 +657,6 @@ Error LLVMOutputStyle::dumpTpiStream(uint32_t StreamIdx) {
   for (const auto &V : Visitors)
     Pipeline.addCallbackToPipeline(*V);
 
-  CVTypeVisitor Visitor(Pipeline);
-
   if (DumpRecords || DumpRecordBytes)
     RecordScope = llvm::make_unique<ListScope>(P, "Records");
 
@@ -673,9 +669,10 @@ Error LLVMOutputStyle::dumpTpiStream(uint32_t StreamIdx) {
     if ((DumpRecords || DumpRecordBytes) && !opts::raw::CompactRecords)
       OneRecordScope = llvm::make_unique<DictScope>(P, "");
 
-    if (auto EC = Visitor.visitTypeRecord(Type))
+    if (auto EC = codeview::visitTypeRecord(Type, Pipeline))
       return EC;
-    T.setIndex(T.getIndex() + 1);
+
+    ++T;
   }
   if (HadError)
     return make_error<RawError>(raw_error_code::corrupt_file,
@@ -730,22 +727,19 @@ Error LLVMOutputStyle::buildTypeDatabase(uint32_t SN) {
 
   DB.emplace(Tpi->getNumTypeRecords());
 
-  TypeVisitorCallbackPipeline Pipeline;
-  TypeDeserializer Deserializer;
   TypeDatabaseVisitor DBV(*DB);
-  Pipeline.addCallbackToPipeline(Deserializer);
-  Pipeline.addCallbackToPipeline(DBV);
 
   auto HashValues = Tpi->getHashValues();
-  std::unique_ptr<TpiHashVerifier> HashVerifier;
-  if (!HashValues.empty()) {
-    HashVerifier =
-        make_unique<TpiHashVerifier>(HashValues, Tpi->getNumHashBuckets());
-    Pipeline.addCallbackToPipeline(*HashVerifier);
-  }
+  if (HashValues.empty())
+    return codeview::visitTypeStream(Tpi->typeArray(), DBV);
 
-  CVTypeVisitor Visitor(Pipeline);
-  return Visitor.visitTypeStream(Tpi->types(nullptr));
+  TypeVisitorCallbackPipeline Pipeline;
+  Pipeline.addCallbackToPipeline(DBV);
+
+  TpiHashVerifier HashVerifier(HashValues, Tpi->getNumHashBuckets());
+  Pipeline.addCallbackToPipeline(HashVerifier);
+
+  return codeview::visitTypeStream(Tpi->typeArray(), Pipeline);
 }
 
 Error LLVMOutputStyle::dumpDbiStream() {

tools/llvm-pdbdump/PdbYaml.cpp

@@ -371,16 +371,14 @@ void MappingContextTraits<PdbInlineeInfo, SerializationContext>::mapping(
 void MappingContextTraits<PdbTpiRecord, pdb::yaml::SerializationContext>::
     mapping(IO &IO, pdb::yaml::PdbTpiRecord &Obj,
             pdb::yaml::SerializationContext &Context) {
-  codeview::TypeVisitorCallbackPipeline Pipeline;
-  codeview::TypeDeserializer Deserializer;
-  codeview::TypeSerializer Serializer(Context.Allocator);
-  pdb::TpiHashUpdater Hasher;
 
   if (IO.outputting()) {
     // For PDB to Yaml, deserialize into a high level record type, then dump it.
-    Pipeline.addCallbackToPipeline(Deserializer);
-    Pipeline.addCallbackToPipeline(Context.Dumper);
+    consumeError(codeview::visitTypeRecord(Obj.Record, Context.Dumper));
   } else {
+    codeview::TypeVisitorCallbackPipeline Pipeline;
+    codeview::TypeSerializer Serializer(Context.Allocator);
+    pdb::TpiHashUpdater Hasher;
     // For Yaml to PDB, extract from the high level record type, then write it
     // to bytes.
 
@@ -391,9 +389,9 @@ void MappingContextTraits<PdbTpiRecord, pdb::yaml::SerializationContext>::
     Pipeline.addCallbackToPipeline(Context.Dumper);
     Pipeline.addCallbackToPipeline(Serializer);
     Pipeline.addCallbackToPipeline(Hasher);
+    consumeError(codeview::visitTypeRecord(Obj.Record, Pipeline,
+                                           codeview::VDS_BytesExternal));
   }
 
-  codeview::CVTypeVisitor Visitor(Pipeline);
-  consumeError(Visitor.visitTypeRecord(Obj.Record));
   Context.ActiveSerializer = nullptr;
 }

tools/llvm-pdbdump/YamlTypeDumper.cpp

@@ -280,16 +280,8 @@ bool ScalarTraits<APSInt>::mustQuote(StringRef Scalar) { return false; }
 
 void MappingContextTraits<CVType, pdb::yaml::SerializationContext>::mapping(
     IO &IO, CVType &Record, pdb::yaml::SerializationContext &Context) {
-  if (IO.outputting()) {
-    codeview::TypeDeserializer Deserializer;
-
-    codeview::TypeVisitorCallbackPipeline Pipeline;
-    Pipeline.addCallbackToPipeline(Deserializer);
-    Pipeline.addCallbackToPipeline(Context.Dumper);
-
-    codeview::CVTypeVisitor Visitor(Pipeline);
-    consumeError(Visitor.visitTypeRecord(Record));
-  }
+  if (IO.outputting())
+    consumeError(codeview::visitTypeRecord(Record, Context.Dumper));
 }
 
 void MappingTraits<StringIdRecord>::mapping(IO &IO, StringIdRecord &String) {
@@ -556,26 +548,17 @@ void llvm::codeview::yaml::YamlTypeDumperCallbacks::visitKnownRecordImpl(
     // (top-level and member fields all have the exact same Yaml syntax so use
     // the same parser).
     FieldListRecordSplitter Splitter(FieldListRecords);
-    CVTypeVisitor V(Splitter);
-    consumeError(V.visitFieldListMemberStream(FieldList.Data));
-    YamlIO.mapRequired("FieldList", FieldListRecords, Context);
-  } else {
-    // If we are not outputting, then the array contains no data starting out,
-    // and is instead populated from the sequence represented by the yaml --
-    // again, using the same logic that we use for top-level records.
-    assert(Context.ActiveSerializer && "There is no active serializer!");
-    codeview::TypeVisitorCallbackPipeline Pipeline;
-    pdb::TpiHashUpdater Hasher;
-
-    // For Yaml to PDB, dump it (to fill out the record fields from the Yaml)
-    // then serialize those fields to bytes, then update their hashes.
-    Pipeline.addCallbackToPipeline(Context.Dumper);
-    Pipeline.addCallbackToPipeline(*Context.ActiveSerializer);
-    Pipeline.addCallbackToPipeline(Hasher);
-
-    codeview::CVTypeVisitor Visitor(Pipeline);
-    YamlIO.mapRequired("FieldList", FieldListRecords, Visitor);
+    consumeError(codeview::visitMemberRecordStream(FieldList.Data, Splitter));
   }
+  // Note that if we're not outputting (i.e. Yaml -> PDB) the result of this
+  // mapping gets lost, as the records are simply stored in this locally scoped
+  // vector.  What's important though is they are all sharing a single
+  // Serializer
+  // instance (in `Context.ActiveSerializer`), and that is building up a list of
+  // all the types.  The fact that we need a throwaway vector here is just to
+  // appease the YAML API to treat this as a sequence and do this mapping once
+  // for each YAML Sequence element in the input Yaml stream.
+  YamlIO.mapRequired("FieldList", FieldListRecords, Context);
 }
 
 namespace llvm {
@@ -585,29 +568,22 @@ struct MappingContextTraits<pdb::yaml::PdbTpiFieldListRecord,
                             pdb::yaml::SerializationContext> {
   static void mapping(IO &IO, pdb::yaml::PdbTpiFieldListRecord &Obj,
                       pdb::yaml::SerializationContext &Context) {
-    assert(IO.outputting());
-    codeview::TypeVisitorCallbackPipeline Pipeline;
+    if (IO.outputting())
+      consumeError(codeview::visitMemberRecord(Obj.Record, Context.Dumper));
+    else {
+      // If we are not outputting, then the array contains no data starting out,
+      // and is instead populated from the sequence represented by the yaml --
+      // again, using the same logic that we use for top-level records.
+      assert(Context.ActiveSerializer && "There is no active serializer!");
+      codeview::TypeVisitorCallbackPipeline Pipeline;
+      pdb::TpiHashUpdater Hasher;
 
-    BinaryByteStream Data(Obj.Record.Data, llvm::support::little);
-    BinaryStreamReader FieldReader(Data);
-    codeview::FieldListDeserializer Deserializer(FieldReader);
-
-    // For PDB to Yaml, deserialize into a high level record type, then dump
-    // it.
-    Pipeline.addCallbackToPipeline(Deserializer);
-    Pipeline.addCallbackToPipeline(Context.Dumper);
-
-    codeview::CVTypeVisitor Visitor(Pipeline);
-    consumeError(Visitor.visitMemberRecord(Obj.Record));
-  }
-};
-
-template <>
-struct MappingContextTraits<pdb::yaml::PdbTpiFieldListRecord,
-                            codeview::CVTypeVisitor> {
-  static void mapping(IO &IO, pdb::yaml::PdbTpiFieldListRecord &Obj,
-                      codeview::CVTypeVisitor &Visitor) {
-    consumeError(Visitor.visitMemberRecord(Obj.Record));
+      Pipeline.addCallbackToPipeline(Context.Dumper);
+      Pipeline.addCallbackToPipeline(*Context.ActiveSerializer);
+      Pipeline.addCallbackToPipeline(Hasher);
+      consumeError(
+          codeview::visitMemberRecord(Obj.Record, Pipeline, VDS_BytesExternal));
+    }
   }
 };
 }

unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp

@@ -12,6 +12,7 @@
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/DebugInfo/CodeView/CVTypeVisitor.h"
 #include "llvm/DebugInfo/CodeView/RandomAccessTypeVisitor.h"
+#include "llvm/DebugInfo/CodeView/TypeDeserializer.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
 #include "llvm/DebugInfo/CodeView/TypeRecordMapping.h"
 #include "llvm/DebugInfo/CodeView/TypeSerializer.h"

unittests/DebugInfo/PDB/TypeServerHandlerTest.cpp

@@ -126,8 +126,8 @@ TEST_F(TypeServerHandlerTest, VisitRecordNoTypeServer) {
 
   Pipeline.addCallbackToPipeline(C1);
   Pipeline.addCallbackToPipeline(C2);
-  CVTypeVisitor Visitor(Pipeline);
-  EXPECT_NO_ERROR(Visitor.visitTypeRecord(TypeServerRecord));
+
+  EXPECT_NO_ERROR(codeview::visitTypeRecord(TypeServerRecord, Pipeline));
 
   EXPECT_EQ(MockTypeVisitorCallbacks::State::VisitTypeEnd, C1.S);
   EXPECT_EQ(MockTypeVisitorCallbacks::State::VisitTypeEnd, C2.S);
@@ -139,16 +139,16 @@ TEST_F(TypeServerHandlerTest, VisitRecordWithTypeServerOnce) {
   MockTypeServerHandler Handler(false);
 
   MockTypeVisitorCallbacks C1;
-  CVTypeVisitor Visitor(C1);
-  Visitor.addTypeServerHandler(Handler);
 
   // Our mock server returns true the first time.
-  EXPECT_NO_ERROR(Visitor.visitTypeRecord(TypeServerRecord));
+  EXPECT_NO_ERROR(codeview::visitTypeRecord(
+      TypeServerRecord, C1, codeview::VDS_BytesExternal, &Handler));
   EXPECT_TRUE(Handler.Handled);
   EXPECT_EQ(MockTypeVisitorCallbacks::State::Ready, C1.S);
 
   // And false the second time.
-  EXPECT_NO_ERROR(Visitor.visitTypeRecord(TypeServerRecord));
+  EXPECT_NO_ERROR(codeview::visitTypeRecord(
+      TypeServerRecord, C1, codeview::VDS_BytesExternal, &Handler));
   EXPECT_TRUE(Handler.Handled);
   EXPECT_EQ(MockTypeVisitorCallbacks::State::VisitTypeEnd, C1.S);
 }
@@ -160,14 +160,14 @@ TEST_F(TypeServerHandlerTest, VisitRecordWithTypeServerAlways) {
   MockTypeServerHandler Handler(true);
 
   MockTypeVisitorCallbacks C1;
-  CVTypeVisitor Visitor(C1);
-  Visitor.addTypeServerHandler(Handler);
 
-  EXPECT_NO_ERROR(Visitor.visitTypeRecord(TypeServerRecord));
+  EXPECT_NO_ERROR(codeview::visitTypeRecord(
+      TypeServerRecord, C1, codeview::VDS_BytesExternal, &Handler));
   EXPECT_TRUE(Handler.Handled);
   EXPECT_EQ(MockTypeVisitorCallbacks::State::Ready, C1.S);
 
-  EXPECT_NO_ERROR(Visitor.visitTypeRecord(TypeServerRecord));
+  EXPECT_NO_ERROR(codeview::visitTypeRecord(
+      TypeServerRecord, C1, codeview::VDS_BytesExternal, &Handler));
   EXPECT_TRUE(Handler.Handled);
   EXPECT_EQ(MockTypeVisitorCallbacks::State::Ready, C1.S);
 }