[Lldb-commits] [PATCH] D60018: [codeview] Remove Type member from CVRecord

[Alexandre Ganea via Phabricator via lldb-commits]

aganea accepted this revision.
aganea added a comment.
This revision is now accepted and ready to land.

LGTM. With a few minor comments:



================
Comment at: llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h:122
   ~FieldListDeserializer() override {
-    CVType FieldList;
-    FieldList.Type = TypeLeafKind::LF_FIELDLIST;
+    RecordPrefix Pre;
+    Pre.RecordLen = 2;
----------------
`RecordPrefix Pre(TypeLeafKind::LF_FIELDLIST);` like the other places? And above.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp:40
+      static CVSymbol Tombstone(
+          ArrayRef<uint8_t>(DenseMapInfo<uint8_t *>::getTombstoneKey(), 1));
       return Tombstone;
----------------
rnk wrote:
> aganea wrote:
> > I suppose you've chosen 1 because that's a invalid record size? Comment maybe?
> 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. :)
Oh I see. Why not `static CVSymbol Tombstone(DenseMapInfo<ArrayRef<uint8_t>>::getTombstoneKey());` then?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60018/new/

https://reviews.llvm.org/D60018