[PATCH] D20717: pdbdump: print out COFF section headers.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: include/llvm/DebugInfo/PDB/Raw/PDBFile.h:79-80
@@ -69,2 +78,4 @@
   Expected<SymbolStream &> getPDBSymbolStream();
+  Expected<codeview::FixedStreamArray<object::coff_section>>
+      getPDBSectionHeaders();
 
----------------
zturner wrote:
> I don't think this should go in `PDBFile`.  Seems to me like it should be accessible from `DBIStream`.
So is publics stream?

================
Comment at: lib/DebugInfo/PDB/Raw/PDBFile.cpp:341
@@ +340,3 @@
+PDBFile::getPDBSectionHeaders() {
+  if (!SectionHeaders) {
+    auto DbiS = getPDBDbiStream();
----------------
zturner wrote:
> It's going to do this each time someone calls the function.  Now that everything is using StreamRef and zero-copy, there is no overhead to creating the `FixedStreamArray` up front, because it's not actually parsing anything until you iterate the collection.  
> 
> So I would move this all to `DBIStream` (as mentioned earlier) and parse this at the time you call `reload`.  Since that doesn't actually parse anything, you can do it once and just store the `FixedStreamArray` inside of `DBIStream` class and return it through a trivial accessor.
Ah, that sounds like a good idea. Will do.


http://reviews.llvm.org/D20717