[PATCH] D25356: Define PDBFileBuilder::addStream to add stream.

[Rui Ueyama via llvm-commits]

ruiu marked an inline comment as done.
ruiu added inline comments.


================
Comment at: include/llvm/DebugInfo/PDB/Raw/DbiStreamBuilder.h:99-101
+  std::vector<support::ulittle16_t> DbgStreams = {(size_t)DbgHeaderType::Max,
+                                                  (support::ulittle16_t)0};
+  std::vector<std::pair<ArrayRef<uint8_t>, uint32_t>> Streams;
----------------
zturner wrote:
> zturner wrote:
> > I wonder if this could just be a `llvm::SmallVector<ArrayRef<uint8_t>, DbgHeaderType::Max>`.  This way you don't need one array which indexes into another array, but rather you just have one array altogether that contains all the data.
> I believe these should be initialized to `kInvalidStreamIndex`.
We need to distinguish an empty stream from nonexistent stream, so we need to store the stream number somewhere. I defined a new class for that purpose.


================
Comment at: lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp:246-249
   uint32_t Length = calculateSerializedLength();
   if (auto EC = Msf.setStreamSize(StreamDBI, Length))
     return EC;
   return Error::success();
----------------
zturner wrote:
> Here you will need to loop through the various optional debug streams and call `setStreamSize()` on each one.
I don't think so. We create a new stream like this

  auto ExpectedIndex = Msf.addStream(Data.size());

and because the size will never change, we don't need to adjust the size afterwards.


https://reviews.llvm.org/D25356