Simplify the use of CVTypeVisitor

[zturner]

Summary

Similar to D33229, this patch is aimed at reducing boilerplate and simplifying usage.

The problem I'm trying to address here is that there is often a lot of code required to visit a type stream, or even a single record. You have to create a visitation pipeline. Then you have to stick a bunch of visitors in it. Then you have to invoke the right method on the pipeline. This is usually between 3 and 6 lines of code, with at least 3 different objects in play. But one of the most common usages is that you have a sequence of bytes, and you want to visit them as deserialized records. In order to make this work, you have to create a TypeVisitorCallbackPipeline and then put a TypeDeserializer on it to deserialize the records, and only then can you add your real callbacks to the visitation pipeline.

Since this is such a common use case, it would be nice if it were automatic. But we don't always want to deserialize records first. We might not even have any record data to deserialize in the first place (think Yaml -> PDB where we have Yaml describing the records, not bytes). So it can't be required.

At the same time, there is no fundamental reason why CVTypeVisitor has to be an object. It just happens to be one. We could just invoke a free free function and pass the visitor into the free function instead of constructing a CVTypeVisitor with one.

So this patch does all of the above. After this patch, the following code:

if (IO.outputting()) {
  codeview::TypeDeserializer Deserializer;

  codeview::TypeVisitorCallbackPipeline Pipeline;
  Pipeline.addCallbackToPipeline(Deserializer);
  Pipeline.addCallbackToPipeline(Context.Dumper);

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

turns into this:

if (IO.outputting())
  consumeError(codeview::visitTypeRecord(Record, Context.Dumper));

[zturner]

Note that there is one instance of using CVTypeVisitor left that I couldn't get rid of in this patch. The one in RandomAccessVisitorTest. I have another patch which depends on this one which makes some fairly significantly change in this area, and after that patch we can get rid of that.

Once that happens, I plan to completely delete CVTypeVisitor (or at least hide it in the implementation file), and the only thing people will use is these newly introduced free functions.

[amccarth]

Having just figured out some of this pipeline stuff, I really appreciate making this easier to user.

llvm/include/llvm/DebugInfo/CodeView/CVTypeVisitor.h

51

I'm not a big fan of bool params, because it can be harder for people reading the call sites to know what they mean. How about making the Deserialize parameter an enum?

[zturner]

Use enum instead of bool.

[amccarth]

I'm happy.

llvm/include/llvm/DebugInfo/CodeView/CVTypeVisitor.h

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

   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
 

llvm/include/llvm/DebugInfo/CodeView/RandomAccessTypeVisitor.h

 #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 {

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

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

   HashTable &getHashAdjusters();
 
   codeview::CVTypeRange types(bool *HadError) const;
+  const codeview::CVTypeArray &typeArray() const { return TypeRecords; }
 
   Error commit();
 

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

       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: ");

llvm/lib/DebugInfo/CodeView/CVTypeDumper.cpp

 #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"

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

llvm/lib/DebugInfo/CodeView/CVTypeVisitor.cpp

   };
 
   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;

   return Error::success();
 }
 
-Error CVTypeVisitor::visitMemberRecord(CVMemberRecord &Record) {
+Error CVTypeVisitor::visitMemberRecord(CVMemberRecord Record) {
   return ::visitMemberRecord(Record, Callbacks);
 }
 

   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);
+}

llvm/lib/DebugInfo/CodeView/RandomAccessTypeVisitor.cpp

 
 #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"

     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());
 }

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

 
   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;
 

llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp

 
 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();

llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp

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

 
 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

     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 &&

llvm/lib/DebugInfo/PDB/Native/PDBTypeServerHandler.cpp

   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;

llvm/tools/llvm-pdbdump/Analyze.cpp

 
   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;

llvm/tools/llvm-pdbdump/LLVMOutputStyle.cpp

 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(

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

   for (const auto &V : Visitors)
     Pipeline.addCallbackToPipeline(*V);
 
-  CVTypeVisitor Visitor(Pipeline);
-
   if (DumpRecords || DumpRecordBytes)
     RecordScope = llvm::make_unique<ListScope>(P, "Records");
 

     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,

 
   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);
+
+  TypeVisitorCallbackPipeline Pipeline;
+  Pipeline.addCallbackToPipeline(DBV);
+
+  TpiHashVerifier HashVerifier(HashValues, Tpi->getNumHashBuckets());
+  Pipeline.addCallbackToPipeline(HashVerifier);
 
-  CVTypeVisitor Visitor(Pipeline);
-  return Visitor.visitTypeStream(Tpi->types(nullptr));
+  return codeview::visitTypeStream(Tpi->typeArray(), Pipeline);
 }
 
 Error LLVMOutputStyle::dumpDbiStream() {

llvm/tools/llvm-pdbdump/PdbYaml.cpp

 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.
 

     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;
 }

llvm/tools/llvm-pdbdump/YamlTypeDumper.cpp

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

     // (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 {

                             pdb::yaml::SerializationContext> {
   static void mapping(IO &IO, pdb::yaml::PdbTpiFieldListRecord &Obj,
                       pdb::yaml::SerializationContext &Context) {
-    assert(IO.outputting());
-    codeview::TypeVisitorCallbackPipeline Pipeline;
-
-    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);
+    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;
+
       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.ActiveSerializer);
+      Pipeline.addCallbackToPipeline(Hasher);
+      consumeError(
+          codeview::visitMemberRecord(Obj.Record, Pipeline, VDS_BytesExternal));
+    }
   }
 };
 }

llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp

 #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"

llvm/unittests/DebugInfo/PDB/TypeServerHandlerTest.cpp

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

   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);
 }

   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);
 }