[PATCH] D22308: [pdb] Introduce an MsfBuilder class for laying out PDB files

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: include/llvm/DebugInfo/PDB/Raw/MsfBuilder.h:33
@@ +32,3 @@
+                  bool CanGrow = true);
+  void reset();
+
----------------
zturner wrote:
> ruiu wrote:
> > What's the use case of `reset()`? You can create as many instances of this class as you want, so wouldn't it be easier to make it a non-reusable class?
> `build()` calls `reset()` after it's done to invalidate the entire structure so you have to start over.  It would be nice if you could call `build()`, make one or two simple changes, then call `build()` again, but it's a little bit tricky for technical reasons, so for now I'm just having `build()` invalidate it.
I think what I was trying to say is to move the code from `reset` to the ctor if needed and remove the `reset` member function entirely. If a user wants to create two or more MSF headers, they can create multiple instances of MsfBuilder, so `reset` does not seem necessary.

================
Comment at: lib/DebugInfo/PDB/Raw/MsfBuilder.cpp:26-27
@@ +25,4 @@
+
+void MsfBuilder::initialize(uint32_t PageSize, uint32_t MinPageCount,
+                            bool CanGrow) {
+  assert(!IsInitialized && "MsfBuilder is already initialized!");
----------------
I think this is a bit error-prone. Why don't you merge this with the ctor?


http://reviews.llvm.org/D22308