[PATCH] D30959: [pdb] Add support for writing Module Info and module symbols

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/ModInfoBuilder.cpp:56
+
+void ModInfoBuilder::setObjFileName(StringRef Name) { ObjFileName = Name; }
+
----------------
inglorion wrote:
> Having this as mutable state increases the surface where bugs can develop. Can we handle this some other way? For example, can we set it in the constructor?
That's the pattern I've been using for all of the other Builder classes.  All of the fields can be set over and over until you call `finalize` / `commit`.  I could perhaps change it, but then that begs the question of what to do about all the other instances where i use this same pattern.  It would be a bit awkward if you had to specify every single value to the constructor, but at the same time there's nothing particularly special about this field over the various other fields.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/ModInfoBuilder.cpp:79
+  Layout.LineBytes = 0;
+  (void)Layout.Mod;         // Set in constructor
+  (void)Layout.ModDiStream; // Set in finalizeMsfLayout
----------------
inglorion wrote:
> What does the comment here mean? What does this line do?
I was trying to make it clear that I wasn't forgetting to set any of the fields in this class, but that some of them should already have been set in other places.


https://reviews.llvm.org/D30959