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

[Rui Ueyama via llvm-commits]

ruiu added inline comments.


================
Comment at: lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp:49
+                                     ArrayRef<uint8_t> Data) {
+  auto ExpectedIndex = Msf.addStream(Data.size());
+  if (!ExpectedIndex)
----------------
zturner wrote:
> I think we shouldn't call `addStream` here.  Suppose someone called `addDbgStream` more than once for the same value of `Type`.  Then you would end up with unreferenced streams in the file.  Maybe instead we can just store the `ArrayRef`, overwriting any previous value of the `ArrayRef` that was there before.  Then in `finalizeMsfLayout` we can add all the streams.
> 
> This way everything is semantically "atomic".  No modification to the underlying file happens until you call `finalize()`, so you can do whatever you want until then without risk of messing anything up.
How about this? I think this is atomic because we check everything that can fail before calling addStream in this function.


https://reviews.llvm.org/D25356