[codeview] Remove Type member from CVRecord

[rnk]

Summary

Now CVType and CVSymbol are effectively type-safe wrappers around
ArrayRef<uint8_t>. Make the kind() accessor load it from the
RecordPrefix, which is the same for types and symbols.

[zturner]

Nice, does this actually have any performance impact? Since the arrays are smaller and have better cache locality.

llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp

370–373

Minor nitpick, but this looks kinda gross to me. How about:

std::vector<CVType> TypeArray = {
  {Builder.records()[0]},
  {Builder.records()[1]}
};

I don't feel strongly though, just a suggestion.

[aganea]

I like when removing code makes a class actually less complex :) (while keeping the same feature set)

llvm/include/llvm/DebugInfo/CodeView/CVRecord.h

36–39

return RecordData.size() >= sizeof(RecordPrefix)

Do we expect an invalid (zero) .RecordKind here?

45

return {};

llvm/include/llvm/DebugInfo/CodeView/SymbolSerializer.h

54–55

This seems like a recurrent pattern, why not add a constructor such as:
CVRecord(RecordPrefix* R) : RecordData((uint8_t*)R, R->RecordLen + 2) {}

Which reduces all this code to:

RecordPrefix R{ulittle16_t(2), ulittle16_t((uint16_t)Sym.Kind)};
CVSymbol Result(&R);

Which could be further improved if RecordPrefix had a RecordPrefix(uint16_t, uint16_t) constructor.

56

Should be 2.

llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h

115

Remove makeFakeFieldList and make this:

RecordPrefix R{ulittle16_t(2), ulittle16_t((uint16_t)TypeLeafKind::LF_FIELDLIST)};
CVSymbol Result(&R);

llvm/lib/DebugInfo/CodeView/ContinuationRecordBuilder.cpp

70

See comment above.

174–175

Same here.

[rnk]

In D60018#1448709, @zturner wrote:

Nice, does this actually have any performance impact? Since the arrays are smaller and have better cache locality.

I haven't done any measurements, actually. I think it will affect the public stream, though, which maintains this vector:

std::vector<CVSymbol> Records;

I got rid of the per-module vector of symbols, though, so I'm not sure how big it will be. I guess I'll just measure heap usage on clang.pdb before I commit.

In D60018#1448983, @aganea wrote:

I like when removing code makes a class actually less complex :) (while keeping the same feature set)

I'm actually wondering if we should remove the size as well, since it's technically redundant with the RecordLen. Originally, while working on the dumper, we used ArrayRef<uint8_t> so that we had a way to make sure we didn't overrun our parent buffer, like an .debug$S subsection. But, I could imagine we might want to validate our inputs up front, and then pass around plain (or wrapped) record pointers in the linker for performance reasons. Hard to say.

Anyway, I agree, this change stands on its own as a good simplification. :)

---

It looks like there's a bunch of new CV code in LLDB that I have to go refactor, so I'll go do that, and then reupload.

llvm/include/llvm/DebugInfo/CodeView/CVRecord.h

36–39

I don't think so.

llvm/include/llvm/DebugInfo/CodeView/SymbolSerializer.h

54–55

In addition to those constructors, I was actually thinking maybe CVRecord should just point to a RecordPrefix* instead of using the ArrayRef wrapper, and then convert back to uint8_t* in .data() and .content().

I tried your suggestion, but I screwed up the field order of RecordPrefix, so I think I'll leave it like this, since it's more self-documenting:

RecordPrefix Prefix;
Prefix.RecordLen = 2;
Prefix.RecordKind = uint16_t(Sym.Kind);
CVSymbol Result(&Prefix, sizeof(Prefix));

I want to pass a size along with the prefix pointer because the whole idea of using ArrayRef in the first place is to maintain the buffer size separately from the input data so we don't end up trusting it.

56

I could say 2, but this is the seralizer, so really it just gets overwritten. Do you think I should leave it uninitialized, 2, -1, 0?

[rnk]

• fix one lldb usage

[aganea]

llvm/include/llvm/DebugInfo/CodeView/CVRecord.h

29

To add to what you said in a comment above, do you think that if we could add assert(Data.size() == ((RecordPrefix *)RecordData.data())->RecordPrefix + 2) at relevant places below; and then after a while we could simply switch to RecordPrefix *, once issues are ironed out?

llvm/include/llvm/DebugInfo/CodeView/SymbolSerializer.h

56

I'd say "2" because you want as much as possible to keep data coherent in time. Regardless of what comes after. I think the code should be correct before being optimal. If someone looks for RecordPrefix and then tries to understand how it works, it'd be nice if the code displays the same, correct, behavior everywhere.

But then, you can argue that RecordPrefix.RecordLen is simply a constrait, tied to the amount data that comes after. And maybe the calculation logic for that .RecordLen field should be hidden inside the RecordPrefix class. In that case, to avoid screwing the init order, ideally you could simply say:

RecordPrefix Prefix{uint16_t(Sym.Kind)};
CVSymbol Result(&Prefix);

...and let RecordPrefix (or CVSymbol) handle sizing?

llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h

125

Applying what I said above would give:

RecordPrefix Pre{uint16_t(TypeLeafKind::LF_FIELDLIST)};
CVType FieldList(&Pre);

llvm/lib/DebugInfo/CodeView/SimpleTypeSerializer.cpp

35

Here it is the same thing: writeRecordPrefix() writes a .RecordLen = 0, but it could very well write 2 to be coherent, then you would be able to trust the RecordPrefix *.

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

40

I suppose you've chosen 1 because that's a invalid record size? Comment maybe?

[aganea]

llvm/include/llvm/DebugInfo/CodeView/CVRecord.h

29

I didn't mean now.

[rnk]

• Add RecordPrefix ctor, remove all dummy RecordLen assignments

[rnk]

• one more change

llvm/include/llvm/DebugInfo/CodeView/CVRecord.h

29

Eventually, yes, I think it's possible.

llvm/include/llvm/DebugInfo/CodeView/SymbolSerializer.h

56

Makes sense. I'll try to standardize on 2, and I like the logic that the data becomes trustworthy.

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

40

Actually, the reason I did this was to avoid ambiguity between the T* begin, T* end ctor and the T* base, size_t count ctor. If I use zero, it's ambiguous. Implicit null strikes again. :)

[aganea]

LGTM. With a few minor comments:

llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h

122

RecordPrefix Pre(TypeLeafKind::LF_FIELDLIST); like the other places? And above.

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

40

Oh I see. Why not static CVSymbol Tombstone(DenseMapInfo<ArrayRef<uint8_t>>::getTombstoneKey()); then?

[rnk]

Thanks!

[rnk]

• final updates

lld/COFF/PDB.cpp

static void translateIdSymbols(MutableArrayRef<uint8_t> &RecordData,
   // In an object file, GPROC32_ID has an embedded reference which refers to the
   // single object file type index namespace.  This has already been translated
   // to the PDB file's ID stream index space, but we need to convert this to a
   // symbol that refers to the type stream index space.  So we remap again from
   // ID index space to type index space.
   if (Kind == SymbolKind::S_GPROC32_ID || Kind == SymbolKind::S_LPROC32_ID) {
     SmallVector<TiReference, 1> Refs;
     auto Content = RecordData.drop_front(sizeof(RecordPrefix));
-    CVSymbol Sym(Kind, RecordData);
+    CVSymbol Sym(RecordData);
     discoverTypeIndicesInSymbol(Sym, Refs);
     assert(Refs.size() == 1);
     assert(Refs.front().Count == 1);
 
     TypeIndex *TI =
         reinterpret_cast<TypeIndex *>(Content.data() + Refs[0].Offset);
     // `TI` is the index of a FuncIdRecord or MemberFuncIdRecord which lives in
     // the IPI stream, whose `FunctionType` member refers to the TPI stream.
void PDBLinker::mergeSymbolRecords(ObjFile *File, const CVIndexMap &IndexMap,
   unsigned CurSymOffset = File->ModuleDBI->getNextSymbolOffset();
   ArrayRef<uint8_t> BulkSymbols;
   cantFail(forEachCodeViewRecord<CVSymbol>(
       SymsBuffer, [&](CVSymbol Sym) -> llvm::Error {
         // Align the record if required.
         MutableArrayRef<uint8_t> RecordBytes;
         if (NeedsRealignment) {
           RecordBytes = copyAndAlignSymbol(Sym, AlignedSymbolMem);
-          Sym = CVSymbol(Sym.kind(), RecordBytes);
+          Sym = CVSymbol(RecordBytes);
         } else {
           // Otherwise, we can actually mutate the symbol directly, since we
           // copied it to apply relocations.
           RecordBytes = makeMutableArrayRef(
               const_cast<uint8_t *>(Sym.data().data()), Sym.length());
         }
 
         // Discover type index references in the record. Skip it if we don't
         // know where they are.
         SmallVector<TiReference, 32> TypeRefs;
         if (!discoverTypeIndicesInSymbol(Sym, TypeRefs)) {
           log("ignoring unknown symbol record with kind 0x" +
               utohexstr(Sym.kind()));
           return Error::success();
         }
 
         // Re-map all the type index references.
         remapTypesInSymbolRecord(File, Sym.kind(), RecordBytes, IndexMap,
                                  TypeRefs);
 
         // An object file may have S_xxx_ID symbols, but these get converted to
         // "real" symbols in a PDB.
         translateIdSymbols(RecordBytes, TMerger.getIDTable());
-        Sym = CVSymbol(symbolKind(RecordBytes), RecordBytes);
+        Sym = CVSymbol(RecordBytes);
 
         // If this record refers to an offset in the object file's string table,
         // add that item to the global PDB string table and re-write the index.
         recordStringTableReferences(Sym.kind(), RecordBytes, StringTableRefs);
 
         // Fill in "Parent" and "End" fields by maintaining a stack of scopes.
         if (symbolOpensScope(Sym.kind()))
           scopeStackOpen(Scopes, CurSymOffset, Sym);

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp

Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr,
   return Error::success();
 }
 
 Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr,
                                            OverloadedMethodRecord &overloaded) {
   TypeIndex method_list_idx = overloaded.MethodList;
 
   CVType method_list_type = m_tpi.getType(method_list_idx);
-  assert(method_list_type.Type == LF_METHODLIST);
+  assert(method_list_type.kind() == LF_METHODLIST);
 
   MethodOverloadListRecord method_list;
   llvm::cantFail(TypeDeserializer::deserializeAs<MethodOverloadListRecord>(
       method_list_type, method_list));
 
   for (const OneMethodRecord &method : method_list.Methods)
     AddMethod(overloaded.Name, method.Type, method.getAccess(),
               method.getOptions(), method.Attrs);

llvm/include/llvm/DebugInfo/CodeView/CVRecord.h

 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include <cstdint>
 
 namespace llvm {
 
 namespace codeview {
 
+/// CVRecord is a fat pointer (base + size pair) to a symbol or type record.
+/// Carrying the size separately instead of trusting the size stored in the
+/// record prefix provides some extra safety and flexibility.
 template <typename Kind> class CVRecord {
 public:
-  CVRecord() : Type(static_cast<Kind>(0)) {}
+  CVRecord() = default;
 
-  CVRecord(Kind K, ArrayRef<uint8_t> Data) : Type(K), RecordData(Data) {}
+  CVRecord(ArrayRef<uint8_t> Data) : RecordData(Data) {}
 
-  bool valid() const { return Type != static_cast<Kind>(0); }
+  CVRecord(const RecordPrefix *P, size_t Size)
+      : RecordData(reinterpret_cast<const uint8_t *>(P), Size) {}
+
+  bool valid() const { return kind() != Kind(0); }
 
   uint32_t length() const { return RecordData.size(); }
-  Kind kind() const { return Type; }
+
+  Kind kind() const {
+    if (RecordData.size() < sizeof(RecordPrefix))
+      return Kind(0);
+    return static_cast<Kind>(static_cast<uint16_t>(
+        reinterpret_cast<const RecordPrefix *>(RecordData.data())->RecordKind));
+  }
+
   ArrayRef<uint8_t> data() const { return RecordData; }
+
   StringRef str_data() const {
     return StringRef(reinterpret_cast<const char *>(RecordData.data()),
                      RecordData.size());
   }
 
   ArrayRef<uint8_t> content() const {
     return RecordData.drop_front(sizeof(RecordPrefix));
   }
 
-  Kind Type;
   ArrayRef<uint8_t> RecordData;
 };
 
 template <typename Kind> struct RemappedRecord {
   explicit RemappedRecord(const CVRecord<Kind> &R) : OriginalRecord(R) {}
 
   CVRecord<Kind> OriginalRecord;
   SmallVector<std::pair<uint32_t, TypeIndex>, 8> Mappings;
while (!StreamBuffer.empty()) {
 
     size_t RealLen = Prefix->RecordLen + 2;
     if (StreamBuffer.size() < RealLen)
       return make_error<CodeViewError>(cv_error_code::corrupt_record);
 
     ArrayRef<uint8_t> Data = StreamBuffer.take_front(RealLen);
     StreamBuffer = StreamBuffer.drop_front(RealLen);
 
-    Record R(static_cast<decltype(Record::Type)>((uint16_t)Prefix->RecordKind),
-             Data);
+    Record R(Data);
     if (auto EC = F(R))
       return EC;
   }
   return Error::success();
 }
 
 /// Read a complete record from a stream at a random offset.
 template <typename Kind>
 inline Expected<CVRecord<Kind>> readCVRecordFromStream(BinaryStreamRef Stream,
                                                        uint32_t Offset) {
   const RecordPrefix *Prefix = nullptr;
   BinaryStreamReader Reader(Stream);
   Reader.setOffset(Offset);
 
   if (auto EC = Reader.readObject(Prefix))
     return std::move(EC);
   if (Prefix->RecordLen < 2)
     return make_error<CodeViewError>(cv_error_code::corrupt_record);
-  Kind K = static_cast<Kind>(uint16_t(Prefix->RecordKind));
 
   Reader.setOffset(Offset);
   ArrayRef<uint8_t> RawData;
   if (auto EC = Reader.readBytes(RawData, Prefix->RecordLen + sizeof(uint16_t)))
     return std::move(EC);
-  return codeview::CVRecord<Kind>(K, RawData);
+  return codeview::CVRecord<Kind>(RawData);
 }
 
 } // end namespace codeview
 
 template <typename Kind>
 struct VarStreamArrayExtractor<codeview::CVRecord<Kind>> {
   Error operator()(BinaryStreamRef Stream, uint32_t &Len,
                    codeview::CVRecord<Kind> &Item) {

llvm/include/llvm/DebugInfo/CodeView/RecordSerialization.h

 using llvm::support::ulittle16_t;
 using llvm::support::ulittle32_t;
 
 /// Limit on the size of all codeview symbol and type records, including the
 /// RecordPrefix. MSVC does not emit any records larger than this.
 enum : unsigned { MaxRecordLength = 0xFF00 };
 
 struct RecordPrefix {
+  RecordPrefix() = default;
+  explicit RecordPrefix(uint16_t Kind) : RecordLen(2), RecordKind(Kind) {}
+
   ulittle16_t RecordLen;  // Record length, starting from &RecordKind.
   ulittle16_t RecordKind; // Record kind enum (SymRecordKind or TypeRecordKind)
 };
 
 /// Reinterpret a byte array as an array of characters. Does not interpret as
 /// a C string, as StringRef has several helpers (split) that make that easy.
 StringRef getBytesAsCharacters(ArrayRef<uint8_t> LeafData);
 StringRef getBytesAsCString(ArrayRef<uint8_t> LeafData);

llvm/include/llvm/DebugInfo/CodeView/SymbolSerializer.h

class SymbolSerializer : public SymbolVisitorCallbacks {
   }
 
 public:
   SymbolSerializer(BumpPtrAllocator &Storage, CodeViewContainer Container);
 
   template <typename SymType>
   static CVSymbol writeOneSymbol(SymType &Sym, BumpPtrAllocator &Storage,
                                  CodeViewContainer Container) {
-    CVSymbol Result;
-    Result.Type = static_cast<SymbolKind>(Sym.Kind);
+    RecordPrefix Prefix{uint16_t(Sym.Kind)};
+    CVSymbol Result(&Prefix, sizeof(Prefix));
     SymbolSerializer Serializer(Storage, Container);
     consumeError(Serializer.visitSymbolBegin(Result));
     consumeError(Serializer.visitKnownRecord(Result, Sym));
     consumeError(Serializer.visitSymbolEnd(Result));
     return Result;
   }
 
   Error visitSymbolBegin(CVSymbol &Record) override;
   Error visitSymbolEnd(CVSymbol &Record) override;

llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h

public:
 
   template <typename T>
   static Expected<T> deserializeAs(ArrayRef<uint8_t> Data) {
     const RecordPrefix *Prefix =
         reinterpret_cast<const RecordPrefix *>(Data.data());
     TypeRecordKind K =
         static_cast<TypeRecordKind>(uint16_t(Prefix->RecordKind));
     T Record(K);
-    CVType CVT(static_cast<TypeLeafKind>(K), Data);
+    CVType CVT(Data);
     if (auto EC = deserializeAs<T>(CVT, Record))
       return std::move(EC);
     return Record;
   }
 
   Error visitTypeBegin(CVType &Record) override {
     assert(!Mapping && "Already in a type mapping!");
     Mapping = llvm::make_unique<MappingInfo>(Record.content());
struct MappingInfo {
 
     BinaryStreamReader &Reader;
     TypeRecordMapping Mapping;
     uint32_t StartOffset;
   };
 
 public:
   explicit FieldListDeserializer(BinaryStreamReader &Reader) : Mapping(Reader) {
-    CVType FieldList;
-    FieldList.Type = TypeLeafKind::LF_FIELDLIST;
+    RecordPrefix Pre;
+    Pre.RecordLen = 2;
+    Pre.RecordKind = uint16_t(TypeLeafKind::LF_FIELDLIST);
+    CVType FieldList(&Pre, sizeof(Pre));
     consumeError(Mapping.Mapping.visitTypeBegin(FieldList));
   }
 
   ~FieldListDeserializer() override {
-    CVType FieldList;
-    FieldList.Type = TypeLeafKind::LF_FIELDLIST;
+    RecordPrefix Pre;
+    Pre.RecordLen = 2;
+    Pre.RecordKind = uint16_t(TypeLeafKind::LF_FIELDLIST);
+    CVType FieldList(&Pre, sizeof(Pre));
     consumeError(Mapping.Mapping.visitTypeEnd(FieldList));
   }
 
   Error visitMemberBegin(CVMemberRecord &Record) override {
     Mapping.StartOffset = Mapping.Reader.getOffset();
     return Mapping.Mapping.visitMemberBegin(Record);
   }
 

llvm/lib/DebugInfo/CodeView/AppendingTypeTableBuilder.cpp

 }
 
 Optional<TypeIndex> AppendingTypeTableBuilder::getNext(TypeIndex Prev) {
   if (++Prev == nextTypeIndex())
     return None;
   return Prev;
 }
 
-CVType AppendingTypeTableBuilder::getType(TypeIndex Index) {
-  CVType Type;
-  Type.RecordData = SeenRecords[Index.toArrayIndex()];
-  const RecordPrefix *P =
-      reinterpret_cast<const RecordPrefix *>(Type.RecordData.data());
-  Type.Type = static_cast<TypeLeafKind>(uint16_t(P->RecordKind));
-  return Type;
+CVType AppendingTypeTableBuilder::getType(TypeIndex Index){
+  return CVType(SeenRecords[Index.toArrayIndex()]);
 }
 
 StringRef AppendingTypeTableBuilder::getTypeName(TypeIndex Index) {
   llvm_unreachable("Method not implemented");
 }
 
 bool AppendingTypeTableBuilder::contains(TypeIndex Index) {
   if (Index.isSimple() || Index.isNoneType())

llvm/lib/DebugInfo/CodeView/CVSymbolVisitor.cpp

 using namespace llvm::codeview;
 
 CVSymbolVisitor::CVSymbolVisitor(SymbolVisitorCallbacks &Callbacks)
     : Callbacks(Callbacks) {}
 
 template <typename T>
 static Error visitKnownRecord(CVSymbol &Record,
                               SymbolVisitorCallbacks &Callbacks) {
-  SymbolRecordKind RK = static_cast<SymbolRecordKind>(Record.Type);
+  SymbolRecordKind RK = static_cast<SymbolRecordKind>(Record.kind());
   T KnownRecord(RK);
   if (auto EC = Callbacks.visitKnownRecord(Record, KnownRecord))
     return EC;
   return Error::success();
 }
 
 static Error finishVisitation(CVSymbol &Record,
                               SymbolVisitorCallbacks &Callbacks) {
-  switch (Record.Type) {
+  switch (Record.kind()) {
   default:
     if (auto EC = Callbacks.visitUnknownSymbol(Record))
       return EC;
     break;
 #define SYMBOL_RECORD(EnumName, EnumVal, Name)                                 \
   case EnumName: {                                                             \
     if (auto EC = visitKnownRecord<Name>(Record, Callbacks))                   \
       return EC;                                                               \

llvm/lib/DebugInfo/CodeView/CVTypeVisitor.cpp

 #include "llvm/Support/BinaryStreamReader.h"
 
 using namespace llvm;
 using namespace llvm::codeview;
 
 
 template <typename T>
 static Error visitKnownRecord(CVType &Record, TypeVisitorCallbacks &Callbacks) {
-  TypeRecordKind RK = static_cast<TypeRecordKind>(Record.Type);
+  TypeRecordKind RK = static_cast<TypeRecordKind>(Record.kind());
   T KnownRecord(RK);
   if (auto EC = Callbacks.visitKnownRecord(Record, KnownRecord))
     return EC;
   return Error::success();
 }
 
 template <typename T>
 static Error visitKnownMember(CVMemberRecord &Record,
private:
   /// The interface to the class that gets notified of each visitation.
   TypeVisitorCallbacks &Callbacks;
 };
 
 CVTypeVisitor::CVTypeVisitor(TypeVisitorCallbacks &Callbacks)
     : Callbacks(Callbacks) {}
 
 Error CVTypeVisitor::finishVisitation(CVType &Record) {
-  switch (Record.Type) {
+  switch (Record.kind()) {
   default:
     if (auto EC = Callbacks.visitUnknownType(Record))
       return EC;
     break;
 #define TYPE_RECORD(EnumName, EnumVal, Name)                                   \
   case EnumName: {                                                             \
     if (auto EC = visitKnownRecord<Name##Record>(Record, Callbacks))           \
       return EC;                                                               \

llvm/lib/DebugInfo/CodeView/ContinuationRecordBuilder.cpp

void ContinuationRecordBuilder::begin(ContinuationRecordKind RecordKind) {
   const SegmentInjection *FLI =
       (RecordKind == ContinuationRecordKind::FieldList)
           ? &InjectFieldList
           : &InjectMethodOverloadList;
   const uint8_t *FLIB = reinterpret_cast<const uint8_t *>(FLI);
   InjectedSegmentBytes =
       ArrayRef<uint8_t>(FLIB, FLIB + sizeof(SegmentInjection));
 
-  CVType Type;
-  Type.Type = getTypeLeafKind(RecordKind);
+  // Seed the first record with an appropriate record prefix.
+  RecordPrefix Prefix(getTypeLeafKind(RecordKind));
+  CVType Type(&Prefix, sizeof(Prefix));
   cantFail(Mapping.visitTypeBegin(Type));
 
-  // Seed the first trecord with an appropriate record prefix.
-  RecordPrefix Prefix;
-  Prefix.RecordLen = 0;
-  Prefix.RecordKind = Type.Type;
   cantFail(SegmentWriter.writeObject(Prefix));
 }
 
 template <typename RecordType>
 void ContinuationRecordBuilder::writeMemberType(RecordType &Record) {
   assert(Kind.hasValue());
 
   uint32_t OriginalOffset = SegmentWriter.getOffset();

 
 CVType ContinuationRecordBuilder::createSegmentRecord(
     uint32_t OffBegin, uint32_t OffEnd, Optional<TypeIndex> RefersTo) {
   assert(OffEnd - OffBegin <= USHRT_MAX);
 
   MutableArrayRef<uint8_t> Data = Buffer.data();
   Data = Data.slice(OffBegin, OffEnd - OffBegin);
 
-  CVType Type;
-  Type.Type = getTypeLeafKind(*Kind);
-  Type.RecordData = Data;
-
   // Write the length to the RecordPrefix, making sure it does not include
   // sizeof(RecordPrefix.Length)
   RecordPrefix *Prefix = reinterpret_cast<RecordPrefix *>(Data.data());
-  assert(Prefix->RecordKind == Type.Type);
   Prefix->RecordLen = Data.size() - sizeof(RecordPrefix::RecordLen);
 
   if (RefersTo.hasValue()) {
     auto Continuation = Data.take_back(ContinuationLength);
     ContinuationRecord *CR =
         reinterpret_cast<ContinuationRecord *>(Continuation.data());
     assert(CR->Kind == TypeLeafKind::LF_INDEX);
     assert(CR->IndexRef == 0xB0C0B0C0);
     CR->IndexRef = RefersTo->getIndex();
   }
 
-  return Type;
+  return CVType(Data);
 }
 
 std::vector<CVType> ContinuationRecordBuilder::end(TypeIndex Index) {
-  CVType Type;
-  Type.Type = getTypeLeafKind(*Kind);
+  RecordPrefix Prefix(getTypeLeafKind(*Kind));
+  CVType Type(&Prefix, sizeof(Prefix));
   cantFail(Mapping.visitTypeEnd(Type));
 
   // We're now done, and we have a series of segments each beginning at an
   // offset specified in the SegmentOffsets array.  We now need to iterate
   // over each segment and post-process them in the following two ways:
   // 1) Each top-level record has a RecordPrefix whose type is either
   //    LF_FIELDLIST or LF_METHODLIST, but the Length field is still 0.
   //    Those should all be set to the correct length now.

llvm/lib/DebugInfo/CodeView/GlobalTypeTableBuilder.cpp

 
 Optional<TypeIndex> GlobalTypeTableBuilder::getNext(TypeIndex Prev) {
   if (++Prev == nextTypeIndex())
     return None;
   return Prev;
 }
 
 CVType GlobalTypeTableBuilder::getType(TypeIndex Index) {
-  CVType Type;
-  Type.RecordData = SeenRecords[Index.toArrayIndex()];
-  if (!Type.RecordData.empty()) {
-    assert(Type.RecordData.size() >= sizeof(RecordPrefix));
-    const RecordPrefix *P =
-        reinterpret_cast<const RecordPrefix *>(Type.RecordData.data());
-    Type.Type = static_cast<TypeLeafKind>(uint16_t(P->RecordKind));
-  }
+  CVType Type(SeenRecords[Index.toArrayIndex()]);
   return Type;
 }
 
 StringRef GlobalTypeTableBuilder::getTypeName(TypeIndex Index) {
   llvm_unreachable("Method not implemented");
 }
 
 bool GlobalTypeTableBuilder::contains(TypeIndex Index) {

llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp

 
 Optional<TypeIndex> MergingTypeTableBuilder::getNext(TypeIndex Prev) {
   if (++Prev == nextTypeIndex())
     return None;
   return Prev;
 }
 
 CVType MergingTypeTableBuilder::getType(TypeIndex Index) {
-  CVType Type;
-  Type.RecordData = SeenRecords[Index.toArrayIndex()];
-  const RecordPrefix *P =
-      reinterpret_cast<const RecordPrefix *>(Type.RecordData.data());
-  Type.Type = static_cast<TypeLeafKind>(uint16_t(P->RecordKind));
+  CVType Type(SeenRecords[Index.toArrayIndex()]);
   return Type;
 }
 
 StringRef MergingTypeTableBuilder::getTypeName(TypeIndex Index) {
   llvm_unreachable("Method not implemented");
 }
 
 bool MergingTypeTableBuilder::contains(TypeIndex Index) {

llvm/lib/DebugInfo/CodeView/SimpleTypeSerializer.cpp

 #include "llvm/DebugInfo/CodeView/SimpleTypeSerializer.h"
 
 using namespace llvm;
 using namespace llvm::codeview;
 
-static void writeRecordPrefix(BinaryStreamWriter &Writer, TypeLeafKind Kind) {
-  RecordPrefix Prefix;
-  Prefix.RecordKind = Kind;
-  Prefix.RecordLen = 0;
-  cantFail(Writer.writeObject(Prefix));
-}
-
 static void addPadding(BinaryStreamWriter &Writer) {
   uint32_t Align = Writer.getOffset() % 4;
   if (Align == 0)
     return;
 
   int PaddingBytes = 4 - Align;
   while (PaddingBytes > 0) {
     uint8_t Pad = static_cast<uint8_t>(LF_PAD0 + PaddingBytes);
     cantFail(Writer.writeInteger(Pad));
     --PaddingBytes;
   }
 }
 
 SimpleTypeSerializer::SimpleTypeSerializer() : ScratchBuffer(MaxRecordLength) {}
 
 SimpleTypeSerializer::~SimpleTypeSerializer() {}
 
 template <typename T>
 ArrayRef<uint8_t> SimpleTypeSerializer::serialize(T &Record) {
   BinaryStreamWriter Writer(ScratchBuffer, support::little);
   TypeRecordMapping Mapping(Writer);
 
-  CVType CVT;
-  CVT.Type = static_cast<TypeLeafKind>(Record.getKind());
+  // Write the record prefix first with a dummy length but real kind.
+  RecordPrefix DummyPrefix(uint16_t(Record.getKind()));
+  cantFail(Writer.writeObject(DummyPrefix));
 
-  writeRecordPrefix(Writer, CVT.Type);
+  RecordPrefix *Prefix = reinterpret_cast<RecordPrefix *>(ScratchBuffer.data());
+  CVType CVT(Prefix, sizeof(RecordPrefix));
 
   cantFail(Mapping.visitTypeBegin(CVT));
   cantFail(Mapping.visitKnownRecord(CVT, Record));
   cantFail(Mapping.visitTypeEnd(CVT));
 
   addPadding(Writer);
 
-  RecordPrefix *Prefix = reinterpret_cast<RecordPrefix *>(ScratchBuffer.data());
-
+  // Update the size and kind after serialization.
   Prefix->RecordKind = CVT.kind();
   Prefix->RecordLen = Writer.getOffset() - sizeof(uint16_t);
 
   return {ScratchBuffer.data(), Writer.getOffset()};
 }
 
 // Explicitly instantiate the member function for each known type so that we can
 // implement this in the cpp file.
 #define TYPE_RECORD(EnumName, EnumVal, Name)                                   \
   template ArrayRef<uint8_t> llvm::codeview::SimpleTypeSerializer::serialize(  \
       Name##Record &Record);
 #define TYPE_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
 #define MEMBER_RECORD(EnumName, EnumVal, Name)
 #define MEMBER_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
 #include "llvm/DebugInfo/CodeView/CodeViewTypes.def"

llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp

void CVSymbolDumperImpl::printLocalVariableAddrGap(
   }
 }
 
 void CVSymbolDumperImpl::printTypeIndex(StringRef FieldName, TypeIndex TI) {
   codeview::printTypeIndex(W, FieldName, TI, Types);
 }
 
 Error CVSymbolDumperImpl::visitSymbolBegin(CVSymbol &CVR) {
-  W.startLine() << getSymbolKindName(CVR.Type);
+  W.startLine() << getSymbolKindName(CVR.kind());
   W.getOStream() << " {\n";
   W.indent();
-  W.printEnum("Kind", unsigned(CVR.Type), getSymbolTypeNames());
+  W.printEnum("Kind", unsigned(CVR.kind()), getSymbolTypeNames());
   return Error::success();
 }
 
 Error CVSymbolDumperImpl::visitSymbolEnd(CVSymbol &CVR) {
   if (PrintRecordBytes && ObjDelegate)
     ObjDelegate->printBinaryBlockWithRelocs("SymData", CVR.content());
 
   W.unindent();

llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp

void TypeDumpVisitor::printItemIndex(StringRef FieldName, TypeIndex TI) const {
   codeview::printTypeIndex(*W, FieldName, TI, getSourceTypes());
 }
 
 Error TypeDumpVisitor::visitTypeBegin(CVType &Record) {
   return visitTypeBegin(Record, TypeIndex::fromArrayIndex(TpiTypes.size()));
 }
 
 Error TypeDumpVisitor::visitTypeBegin(CVType &Record, TypeIndex Index) {
-  W->startLine() << getLeafTypeName(Record.Type);
+  W->startLine() << getLeafTypeName(Record.kind());
   W->getOStream() << " (" << HexNumber(Index.getIndex()) << ")";
   W->getOStream() << " {\n";
   W->indent();
-  W->printEnum("TypeLeafKind", unsigned(Record.Type),
+  W->printEnum("TypeLeafKind", unsigned(Record.kind()),
                makeArrayRef(LeafTypeNames));
   return Error::success();
 }
 
 Error TypeDumpVisitor::visitTypeEnd(CVType &Record) {
   if (PrintRecordBytes)
     W->printBinaryBlock("LeafData", getBytesAsCharacters(Record.content()));
 

llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp

 Error TypeRecordMapping::visitTypeBegin(CVType &CVR) {
   assert(!TypeKind.hasValue() && "Already in a type mapping!");
   assert(!MemberKind.hasValue() && "Already in a member mapping!");
 
   // FieldList and MethodList records can be any length because they can be
   // split with continuation records.  All other record types cannot be
   // longer than the maximum record length.
   Optional<uint32_t> MaxLen;
-  if (CVR.Type != TypeLeafKind::LF_FIELDLIST &&
-      CVR.Type != TypeLeafKind::LF_METHODLIST)
+  if (CVR.kind() != TypeLeafKind::LF_FIELDLIST &&
+      CVR.kind() != TypeLeafKind::LF_METHODLIST)
     MaxLen = MaxRecordLength - sizeof(RecordPrefix);
   error(IO.beginRecord(MaxLen));
-  TypeKind = CVR.Type;
+  TypeKind = CVR.kind();
   return Error::success();
 }
 
 Error TypeRecordMapping::visitTypeEnd(CVType &Record) {
   assert(TypeKind.hasValue() && "Not in a type mapping!");
   assert(!MemberKind.hasValue() && "Still in a member mapping!");
 
   error(IO.endRecord());
Error TypeRecordMapping::visitKnownRecord(CVType &CVR, ArrayRecord &Record) {
   error(IO.mapInteger(Record.IndexType));
   error(IO.mapEncodedInteger(Record.Size));
   error(IO.mapStringZ(Record.Name));
 
   return Error::success();
 }
 
 Error TypeRecordMapping::visitKnownRecord(CVType &CVR, ClassRecord &Record) {
-  assert((CVR.Type == TypeLeafKind::LF_STRUCTURE) ||
-         (CVR.Type == TypeLeafKind::LF_CLASS) ||
-         (CVR.Type == TypeLeafKind::LF_INTERFACE));
+  assert((CVR.kind() == TypeLeafKind::LF_STRUCTURE) ||
+         (CVR.kind() == TypeLeafKind::LF_CLASS) ||
+         (CVR.kind() == TypeLeafKind::LF_INTERFACE));
 
   error(IO.mapInteger(Record.MemberCount));
   error(IO.mapEnum(Record.Options));
   error(IO.mapInteger(Record.FieldList));
   error(IO.mapInteger(Record.DerivationList));
   error(IO.mapInteger(Record.VTableShape));
   error(IO.mapEncodedInteger(Record.Size));
   error(mapNameAndUniqueName(IO, Record.Name, Record.UniqueName,

llvm/lib/DebugInfo/CodeView/TypeTableCollection.cpp

Optional<TypeIndex> TypeTableCollection::getNext(TypeIndex Prev) {
   ++Prev;
   if (Prev.toArrayIndex() == size())
     return None;
   return Prev;
 }
 
 CVType TypeTableCollection::getType(TypeIndex Index) {
   assert(Index.toArrayIndex() < Records.size());
-  ArrayRef<uint8_t> Bytes = Records[Index.toArrayIndex()];
-  const RecordPrefix *Prefix =
-      reinterpret_cast<const RecordPrefix *>(Bytes.data());
-  TypeLeafKind Kind = static_cast<TypeLeafKind>(uint16_t(Prefix->RecordKind));
-  return CVType(Kind, Bytes);
+  return CVType(Records[Index.toArrayIndex()]);
 }
 
 StringRef TypeTableCollection::getTypeName(TypeIndex Index) {
   if (Index.isNoneType() || Index.isSimple())
     return TypeIndex::simpleTypeName(Index);
 
   uint32_t I = Index.toArrayIndex();
   if (Names[I].data() == nullptr) {

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

 
 struct llvm::pdb::GSIHashStreamBuilder {
   struct UdtDenseMapInfo {
     static inline CVSymbol getEmptyKey() {
       static CVSymbol Empty;
       return Empty;
     }
     static inline CVSymbol getTombstoneKey() {
-      static CVSymbol Tombstone(static_cast<SymbolKind>(-1),
-                                ArrayRef<uint8_t>());
+      static CVSymbol Tombstone(
+          makeArrayRef(DenseMapInfo<uint8_t *>::getTombstoneKey(), 0));
       return Tombstone;
     }
     static unsigned getHashValue(const CVSymbol &Val) {
       return xxHash64(Val.RecordData);
     }
     static bool isEqual(const CVSymbol &LHS, const CVSymbol &RHS) {
       return LHS.RecordData == RHS.RecordData;
     }

llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp

CVSymbol toCodeViewSymbol(BumpPtrAllocator &Allocator,
                             CodeViewContainer Container) const override {
     RecordPrefix Prefix;
     uint32_t TotalLen = sizeof(RecordPrefix) + Data.size();
     Prefix.RecordKind = Kind;
     Prefix.RecordLen = TotalLen - 2;
     uint8_t *Buffer = Allocator.Allocate<uint8_t>(TotalLen);
     ::memcpy(Buffer, &Prefix, sizeof(RecordPrefix));
     ::memcpy(Buffer + sizeof(RecordPrefix), Data.data(), Data.size());
-    return CVSymbol(Kind, ArrayRef<uint8_t>(Buffer, TotalLen));
+    return CVSymbol(ArrayRef<uint8_t>(Buffer, TotalLen));
   }
 
   Error fromCodeViewSymbol(CVSymbol CVS) override {
     this->Kind = CVS.kind();
     Data = CVS.RecordData.drop_front(sizeof(RecordPrefix));
     return Error::success();
   }
 

llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp

template <typename T> struct LeafRecordImpl : public LeafRecordBase {
   void map(yaml::IO &io) override;
 
   Error fromCodeViewRecord(CVType Type) override {
     return TypeDeserializer::deserializeAs<T>(Type, Record);
   }
 
   CVType toCodeViewRecord(AppendingTypeTableBuilder &TS) const override {
     TS.writeLeafType(Record);
-    return CVType(Kind, TS.records().back());
+    return CVType(TS.records().back());
   }
 
   mutable T Record;
 };
 
 template <> struct LeafRecordImpl<FieldListRecord> : public LeafRecordBase {
   explicit LeafRecordImpl(TypeLeafKind K) : LeafRecordBase(K) {}
 

 CVType LeafRecordImpl<FieldListRecord>::toCodeViewRecord(
     AppendingTypeTableBuilder &TS) const {
   ContinuationRecordBuilder CRB;
   CRB.begin(ContinuationRecordKind::FieldList);
   for (const auto &Member : Members) {
     Member.Member->writeTo(CRB);
   }
   TS.insertRecord(CRB);
-  return CVType(Kind, TS.records().back());
+  return CVType(TS.records().back());
 }
 
 void MappingTraits<OneMethodRecord>::mapping(IO &io, OneMethodRecord &Record) {
   io.mapRequired("Type", Record.Type);
   io.mapRequired("Attrs", Record.Attrs.Attrs);
   io.mapRequired("VFTableOffset", Record.VFTableOffset);
   io.mapRequired("Name", Record.Name);
 }

llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp

 
 Error MinimalSymbolDumper::visitSymbolBegin(codeview::CVSymbol &Record,
                                             uint32_t Offset) {
   // formatLine puts the newline at the beginning, so we use formatLine here
   // to start a new line, and then individual visit methods use format to
   // append to the existing line.
   P.formatLine("{0} | {1} [size = {2}]",
                fmt_align(Offset, AlignStyle::Right, 6),
-               formatSymbolKind(Record.Type), Record.length());
+               formatSymbolKind(Record.kind()), Record.length());
   P.Indent();
   return Error::success();
 }
 
 Error MinimalSymbolDumper::visitSymbolEnd(CVSymbol &Record) {
   if (RecordBytes) {
     AutoIndent Indent(P, 7);
     P.formatBinary("bytes", Record.content(), 0);

llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp

 
 Error MinimalTypeDumpVisitor::visitTypeBegin(CVType &Record, TypeIndex Index) {
   CurrentTypeIndex = Index;
   // formatLine puts the newline at the beginning, so we use formatLine here
   // to start a new line, and then individual visit methods use format to
   // append to the existing line.
   P.formatLine("{0} | {1} [size = {2}",
                fmt_align(Index, AlignStyle::Right, Width),
-               formatTypeLeafKind(Record.Type), Record.length());
+               formatTypeLeafKind(Record.kind()), Record.length());
   if (Hashes) {
     std::string H;
     if (Index.toArrayIndex() >= HashValues.size()) {
       H = "(not present)";
     } else {
       uint32_t Hash = HashValues[Index.toArrayIndex()];
       Expected<uint32_t> MaybeHash = hashTypeRecord(Record);
       if (!MaybeHash)

llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp

if (R1.Size != R2.Size)
     return false;
   return true;
 }
 inline bool operator!=(const ArrayRecord &R1, const ArrayRecord &R2) {
   return !(R1 == R2);
 }
 
 inline bool operator==(const CVType &R1, const CVType &R2) {
-  if (R1.Type != R2.Type)
-    return false;
   if (R1.RecordData != R2.RecordData)
     return false;
   return true;
 }
 inline bool operator!=(const CVType &R1, const CVType &R2) {
   return !(R1 == R2);
 }
 }
for (int I = 0; I < 11; ++I) {
       AR.Size = I;
       std::string Name;
       raw_string_ostream Stream(Name);
       Stream << "Array [" << I << "]";
       AR.Name = GlobalState->Strings.save(Stream.str());
       GlobalState->Records.push_back(AR);
       GlobalState->Indices.push_back(Builder.writeLeafType(AR));
 
-      CVType Type(TypeLeafKind::LF_ARRAY, Builder.records().back());
+      CVType Type(Builder.records().back());
       GlobalState->TypeVector.push_back(Type);
 
       GlobalState->AllOffsets.push_back(
           {GlobalState->Indices.back(), ulittle32_t(Offset)});
       Offset += Type.length();
     }
 
     GlobalState->ItemStream.setItems(GlobalState->TypeVector);
TEST_F(RandomAccessVisitorTest, CrossChunkName) {
 
   // TypeIndex 1 refers to type index 0.
   ModifierRecord Modifier(TypeRecordKind::Modifier);
   Modifier.ModifiedType = TypeIndex::fromArrayIndex(0);
   Modifier.Modifiers = ModifierOptions::Const;
   TypeIndex IndexOne = Builder.writeLeafType(Modifier);
 
   // set up a type stream that refers to the above two serialized records.
-  std::vector<CVType> TypeArray;
-  TypeArray.push_back(
-      CVType(static_cast<TypeLeafKind>(Class.Kind), Builder.records()[0]));
-  TypeArray.push_back(
-      CVType(static_cast<TypeLeafKind>(Modifier.Kind), Builder.records()[1]));
+  std::vector<CVType> TypeArray = {
+      {Builder.records()[0]},
+      {Builder.records()[1]},
+  };
   BinaryItemStream<CVType> ItemStream(llvm::support::little);
   ItemStream.setItems(TypeArray);
   VarStreamArray<CVType> TypeStream(ItemStream);
 
   // Figure out the byte offset of the second item.
   auto ItemOneIter = TypeStream.begin();
   ++ItemOneIter;