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 ↗

(On Diff #99163)

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/trunk/include/llvm/DebugInfo/CodeView/CVTypeVisitor.h

 class CVTypeVisitor {
 public:
   explicit CVTypeVisitor(TypeVisitorCallbacks &Callbacks);
 
   void addTypeServerHandler(TypeServerHandler &Handler);
 
   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);
   Error visitTypeStream(CVTypeRange Types);
 
   Error visitFieldListMemberStream(ArrayRef<uint8_t> FieldList);
   Error visitFieldListMemberStream(BinaryStreamReader Reader);
 
 private:
   Expected<bool> handleTypeServer(CVType &Record);
   Error finishVisitation(CVType &Record);
 
   /// The interface to the class that gets notified of each visitation.
   TypeVisitorCallbacks &Callbacks;
 
   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
 
 #endif // LLVM_DEBUGINFO_CODEVIEW_CVTYPEVISITOR_H

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

 //===- RandomAccessTypeVisitor.h ------------------------------ *- C++ --*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef 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 {
 namespace codeview {
 
 class TypeDatabase;
 class TypeServerHandler;
 class TypeVisitorCallbacks;
private:
   TypeDatabase Database;
 
   /// The type array to allow random access visitation of.
   const CVTypeArray &Types;
 
   /// 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.
   std::vector<uint32_t> KnownOffsets;
 
   /// An array of index offsets for the given type stream, allowing log(N)
   /// lookups of a type record by index.  Similar to KnownOffsets but only
   /// contains offsets for some type indices, some of which may not have
   /// ever been visited.
   PartialOffsetArray PartialOffsets;
 };
 
 } // end namespace codeview
 } // end namespace llvm
 
 #endif // LLVM_DEBUGINFO_CODEVIEW_RANDOMACCESSTYPEVISITOR_H

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

public:
 
   uint32_t getHashKeySize() const;
   uint32_t getNumHashBuckets() const;
   FixedStreamArray<support::ulittle32_t> getHashValues() const;
   FixedStreamArray<codeview::TypeIndexOffset> getTypeIndexOffsets() const;
   HashTable &getHashAdjusters();
 
   codeview::CVTypeRange types(bool *HadError) const;
+  const codeview::CVTypeArray &typeArray() const { return TypeRecords; }
 
   Error commit();
 
 private:
   const PDBFile &Pdb;
   std::unique_ptr<msf::MappedBlockStream> Stream;
 
   codeview::CVTypeArray TypeRecords;

llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

#ifndef NDEBUG
       // so the first link of an invalid type record can succeed while
       // subsequent links will fail with LNK1285.
       BinaryByteStream Stream(Record, llvm::support::little);
       CVTypeArray Types;
       BinaryStreamReader Reader(Stream);
       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_unreachable("produced malformed type record");
       }
 #endif
     }
     StringRef S(reinterpret_cast<const char *>(Record.data()), Record.size());

llvm/trunk/lib/DebugInfo/CodeView/CVTypeDumper.cpp

 //===-- CVTypeDumper.cpp - CodeView type info dumper ------------*- 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/CVTypeDumper.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/TypeRecord.h"
 #include "llvm/DebugInfo/CodeView/TypeVisitorCallbackPipeline.h"
 #include "llvm/Support/BinaryByteStream.h"
 
 using namespace llvm;
 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) {
   BinaryByteStream Stream(Data, llvm::support::little);
   CVTypeArray Types;
   BinaryStreamReader Reader(Stream);
   if (auto EC = Reader.readArray(Types, Reader.getLength()))
     return EC;

llvm/trunk/lib/DebugInfo/CodeView/CVTypeVisitor.cpp

Error visitKnownRecord(CVType &CVR, TypeServer2Record &Record) override {
       return Error::success();
     }
 
   private:
     TypeServer2Record &TR;
   };
 
   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;
 }
 
 void CVTypeVisitor::addTypeServerHandler(TypeServerHandler &Handler) {
   Handlers.push_back(&Handler);
 }
#include "llvm/DebugInfo/CodeView/TypeRecords.def"
   }
 
   if (auto EC = Callbacks.visitMemberEnd(Record))
     return EC;
 
   return Error::success();
 }
 
-Error CVTypeVisitor::visitMemberRecord(CVMemberRecord &Record) {
+Error CVTypeVisitor::visitMemberRecord(CVMemberRecord Record) {
   return ::visitMemberRecord(Record, Callbacks);
 }
 
 /// Visits the type records in Data. Sets the error flag on parse failures.
 Error CVTypeVisitor::visitTypeStream(const CVTypeArray &Types) {
   for (auto I : Types) {
     if (auto EC = visitTypeRecord(I))
       return EC;
Error CVTypeVisitor::visitFieldListMemberStream(BinaryStreamReader Reader) {
   return Error::success();
 }
 
 Error CVTypeVisitor::visitFieldListMemberStream(ArrayRef<uint8_t> Data) {
   BinaryByteStream S(Data, llvm::support::little);
   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/trunk/lib/DebugInfo/CodeView/RandomAccessTypeVisitor.cpp

 //===- RandomAccessTypeVisitor.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/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"
 
 using namespace llvm;
 using namespace llvm::codeview;
 
 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());
 }
 
 Error RandomAccessTypeVisitor::visitTypeIndex(TypeIndex TI,
                                               TypeVisitorCallbacks &Callbacks) {
   assert(TI.toArrayIndex() < Database.capacity());
 
   if (!Database.contains(TI)) {
     if (auto EC = visitRangeForType(TI))
       return EC;
   }
 
   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) {
   if (PartialOffsets.empty()) {
     TypeIndex TIB(TypeIndex::FirstNonSimpleIndex);
     TypeIndex TIE = TIB + Database.capacity();
     return visitRange(TIB, 0, TIE);
   }

 Error RandomAccessTypeVisitor::visitRange(TypeIndex Begin, uint32_t BeginOffset,
                                           TypeIndex End) {
 
   auto RI = Types.at(BeginOffset);
   assert(RI != Types.end());
 
   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;
 
     BeginOffset += RI.getRecordLength();
     ++Begin;
     ++RI;
   }
 
   return Error::success();
 }

llvm/trunk/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp

Error TypeDumpVisitor::visitMemberEnd(CVMemberRecord &Record) {
 
   W->unindent();
   W->startLine() << "}\n";
   return Error::success();
 }
 
 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();
 }
 
 Error TypeDumpVisitor::visitKnownRecord(CVType &CVR, StringIdRecord &String) {
   printItemIndex("Id", String.getId());
   W->printString("StringData", String.getString());

llvm/trunk/lib/DebugInfo/CodeView/TypeStreamMerger.cpp

for (OneMethodRecord &Meth : R.Methods)
     Success &= remapIndex(Meth.Type);
   return writeRecord(R, Success);
 }
 
 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.
   TypeIndex DestIdx = Untranslated;
   if (!HadUntranslatedMember)
     DestIdx = FieldListBuilder.end();
   else
     FieldListBuilder.reset();

 Error TypeStreamMerger::visitUnknownType(CVType &Rec) {
   // We failed to translate a type. Translate this index as "not translated".
   addMapping(TypeIndex(SimpleTypeKind::NotTranslated));
   return errorCorruptRecord();
 }
 
 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
   // if we can resolve the indices that weren't in the map on the first pass.
   // This may require multiple passes, but we should always make progress. MASM
   // is the only known CodeView producer that makes type streams that aren't
   // topologically sorted. The standard library contains MASM-produced objects,
   // so this is important to handle correctly, but we don't have to be too
   // efficient. MASM type streams are usually very small.
   while (!*LastError && NumBadIndices > 0) {
     unsigned BadIndicesRemaining = NumBadIndices;
     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 &&
            "second pass found more bad indices");
     if (!*LastError && NumBadIndices == BadIndicesRemaining) {
       return llvm::make_error<CodeViewError>(
           cv_error_code::corrupt_record, "input type graph contains cycles");
     }

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

 }
 
 Expected<bool>
 PDBTypeServerHandler::handleInternal(PDBFile &File,
                                      TypeVisitorCallbacks &Callbacks) {
   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;
 }
 
 Expected<bool> PDBTypeServerHandler::handle(TypeServer2Record &TS,
                                             TypeVisitorCallbacks &Callbacks) {
   if (Session) {

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

 
 Error AnalysisStyle::dump() {
   auto Tpi = File.getPDBTpiStream();
   if (!Tpi)
     return Tpi.takeError();
 
   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))
 
   auto &Adjusters = Tpi->getHashAdjusters();
   DenseSet<uint32_t> AdjusterSet;
   for (const auto &Adj : Adjusters) {
     assert(AdjusterSet.find(Adj.second) == AdjusterSet.end());
     AdjusterSet.insert(Adj.second);
   }
 

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

for (const auto &IL : InlineeLines) {
       }
     }
     return Error::success();
   }
 
 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(
           llvm::formatv("Index: {0:x} (unknown function)", Index.getIndex())
               .str());
     }
     return Error::success();
   }
Error LLVMOutputStyle::dumpTpiStream(uint32_t StreamIdx) {
   StreamScope = llvm::make_unique<DictScope>(P, Label);
   P.printNumber(VerLabel, Tpi->getTpiVersion());
   P.printNumber("Record count", Tpi->getNumTypeRecords());
 
   Optional<TypeDatabase> &StreamDB = (StreamIdx == StreamTPI) ? TypeDB : ItemDB;
 
   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));
   }
   // If we're in dump mode, add a dumper with the appropriate detail level.
   if (DumpRecords) {
     std::unique_ptr<TypeVisitorCallbacks> Dumper;
     if (opts::raw::CompactRecords)
Error LLVMOutputStyle::dumpTpiStream(uint32_t StreamIdx) {
 
   // We always need to deserialize and add it to the type database.  This is
   // true if even if we're not dumping anything, because we could need the
   // type database for the purposes of dumping symbols.
   TypeVisitorCallbackPipeline Pipeline;
   for (const auto &V : Visitors)
     Pipeline.addCallbackToPipeline(*V);
 
-  CVTypeVisitor Visitor(Pipeline);
-
   if (DumpRecords || DumpRecordBytes)
     RecordScope = llvm::make_unique<ListScope>(P, "Records");
 
   bool HadError = false;
 
   TypeIndex T(TypeIndex::FirstNonSimpleIndex);
   for (auto Type : Tpi->types(&HadError)) {
     std::unique_ptr<DictScope> OneRecordScope;
 
     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,
                                 "TPI stream contained corrupt record");
 
   if (DumpTpiHash) {
     DictScope DD(P, "Hash");
     P.printNumber("Number of Hash Buckets", Tpi->getNumHashBuckets());
Error LLVMOutputStyle::buildTypeDatabase(uint32_t SN) {
   auto Tpi =
       (SN == StreamTPI) ? File.getPDBTpiStream() : File.getPDBIpiStream();
 
   if (!Tpi)
     return Tpi.takeError();
 
   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() {
   bool DumpModules = opts::raw::DumpModules || opts::raw::DumpModuleSyms ||
                      opts::raw::DumpModuleFiles || opts::raw::DumpLineInfo;
   if (!opts::raw::DumpHeaders && !DumpModules)
     return Error::success();
   if (!File.hasPDBDbiStream()) {

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

void MappingContextTraits<PdbInlineeInfo, SerializationContext>::mapping(
     IO &IO, PdbInlineeInfo &Obj, SerializationContext &Context) {
   IO.mapRequired("HasExtraFiles", Obj.HasExtraFiles);
   IO.mapRequired("Sites", Obj.Sites, Context);
 }
 
 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.
 
     // This might be interpreted as a hack, but serializing FieldList
     // sub-records requires having access to the same serializer being used by
     // the FieldList itself.
     Context.ActiveSerializer = &Serializer;
     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/trunk/tools/llvm-pdbdump/YamlTypeDumper.cpp

StringRef ScalarTraits<APSInt>::input(StringRef Scalar, void *Ctx, APSInt &S) {
   S = APSInt(Scalar);
   return "";
 }
 
 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) {
   IO.mapRequired("Id", String.Id);
   IO.mapRequired("String", String.String);
 }
 
 void MappingTraits<ArgListRecord>::mapping(IO &IO, ArgListRecord &Args) {
if (YamlIO.outputting()) {
     // If we are outputting, then `FieldList.Data` contains a huge chunk of data
     // representing the serialized list of members.  We need to split it up into
     // individual CVType records where each record represents an individual
     // member.  This way, we can simply map the entire thing as a Yaml sequence,
     // which will recurse back to the standard handler for top-level fields
     // (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 {
 namespace yaml {
 template <>
 struct MappingContextTraits<pdb::yaml::PdbTpiFieldListRecord,
                             pdb::yaml::SerializationContext> {
   static void mapping(IO &IO, pdb::yaml::PdbTpiFieldListRecord &Obj,
                       pdb::yaml::SerializationContext &Context) {
-    assert(IO.outputting());
+    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));
+      Pipeline.addCallbackToPipeline(*Context.ActiveSerializer);
+      Pipeline.addCallbackToPipeline(Hasher);
+      consumeError(
+          codeview::visitMemberRecord(Obj.Record, Pipeline, VDS_BytesExternal));
     }
-};
-
-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));
   }
 };
 }
 }

llvm/trunk/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp

 //===- llvm/unittest/DebugInfo/CodeView/RandomAccessVisitorTest.cpp -------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "ErrorChecking.h"
 
 #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"
 #include "llvm/DebugInfo/CodeView/TypeServerHandler.h"
 #include "llvm/DebugInfo/CodeView/TypeTableBuilder.h"
 #include "llvm/DebugInfo/CodeView/TypeVisitorCallbackPipeline.h"
 #include "llvm/DebugInfo/CodeView/TypeVisitorCallbacks.h"
 #include "llvm/DebugInfo/PDB/Native/RawTypes.h"

llvm/trunk/unittests/DebugInfo/PDB/TypeServerHandlerTest.cpp

 // visitor callbacks.
 TEST_F(TypeServerHandlerTest, VisitRecordNoTypeServer) {
   MockTypeVisitorCallbacks C2;
   MockTypeVisitorCallbacks C1;
   TypeVisitorCallbackPipeline Pipeline;
 
   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);
 }
 
 // Test that when a TypeServerHandler is registered, it gets consumed by the
 // handler if and only if the handler returns true.
 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);
 }
 
 // Test that when a type server handler is registered, if the handler keeps
 // returning true, it will keep getting consumed by the handler and not go
 // to the default processor.
 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);
 }
 
 } // end anonymous namespace