[PATCH] D19634: Read the rest of the substreams from DBI, and parse source file information

[David Majnemer via llvm-commits]

majnemer added a subscriber: majnemer.

================
Comment at: include/llvm/DebugInfo/PDB/Raw/ModInfo.h:47
@@ +46,3 @@
+struct ModuleInfoEx {
+  ModuleInfoEx(ModInfo Module) : Info(Module) {}
+
----------------
Maybe make the parameter be a `const &`

================
Comment at: lib/DebugInfo/PDB/Raw/PDBDbiStream.cpp:51
@@ -50,4 +50,3 @@
   ulittle32_t Age; // Should match PDBInfoStream.
-  ulittle16_t GSSyms;
-  ulittle16_t BuildNumber; // See DbiBuildNo structure.
-  ulittle16_t PSSyms;
+  ulittle16_t GSSyms;               // Number of global symbols
+  ulittle16_t BuildNumber;          // See DbiBuildNo structure.
----------------
Would it be easier to decode this field if it were named something like `NumGlobalSyms` or `NumGlobalSymbols`?

================
Comment at: lib/DebugInfo/PDB/Raw/PDBDbiStream.cpp:191
@@ +190,3 @@
+  Bytes.resize(Size);
+  return Stream.readBytes(&Bytes[0], Size);
+}
----------------
I'd be nice if the readBytes interface took a `MutableArrayRef<uint8_t>`.  Then you'd just pass in the `std::vector`.

================
Comment at: lib/DebugInfo/PDB/Raw/PDBDbiStream.cpp:216-218
@@ +215,5 @@
+  //
+  const uint8_t *Buf = &FileInfoSubstream[0];
+  auto FI = reinterpret_cast<const FileInfoSubstreamHeader *>(Buf);
+  Buf += sizeof(FileInfoSubstreamHeader);
+  // The number of modules in the stream should be the same as reported by
----------------
Couldn't you also do:
  auto *Buf = reinterpret_cast<const ulittle16_t *>(&FileInfoSubstream[1]);


http://reviews.llvm.org/D19634