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

[Zachary Turner via llvm-commits]

zturner added inline comments.


================
Comment at: include/llvm/DebugInfo/PDB/Raw/DbiStreamBuilder.h:99-100
   msf::MutableByteStream FileInfoBuffer;
+  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:
> 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`.


================
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;
----------------
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.


================
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();
----------------
Here you will need to loop through the various optional debug streams and call `setStreamSize()` on each one.


================
Comment at: lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp:295-296
+    uint32_t Idx = Pair.second;
+    auto Stream =
+        WritableMappedBlockStream::createIndexedStream(Layout, Buffer, Idx);
+    StreamWriter Writer(*Stream);
----------------
Here you should check if `Idx == kInvalidStreamIndex`, and `continue` if so.


================
Comment at: lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp:297
+        WritableMappedBlockStream::createIndexedStream(Layout, Buffer, Idx);
+    StreamWriter Writer(*Stream);
+    if (auto EC = Writer.writeArray(Data))
----------------
Can you use a different variable name here?  This `Writer` shadows the previous `Writer`, which could be confusing.  Maybe calling it `DbgStreamWriter`.


https://reviews.llvm.org/D25356