[PATCH] D19570: [PDB] Parse module information from the DBI stream

[Zachary Turner via llvm-commits]

zturner marked 3 inline comments as done.

================
Comment at: lib/DebugInfo/PDB/Raw/ModInfo.cpp:53-56
@@ +52,6 @@
+  ulittle16_t NumFiles;      // Number of files contributing to this module
+  ulittle32_t FileNameOffsets; // array of [0..NumFiles) DBI name buffer offsets
+                               // This field is a pointer in the reference
+                               // implementation, but as with `Mod`, we ignore
+                               // it for now since it is unused.
+  ulittle32_t SrcFileNameNI; // Name Index for src file name
----------------
majnemer wrote:
> Won't it be 64-bit in size on a 64-bit system?
Not as far as I can tell.  They have 2 different structures that are almost the same, one using a uint32 and one using a pointer.  The former is the file layout format, and the latter is an in-memory format.  But since we aren't using these fields yet anyway, it doesn't make sense to add the additional complexity, as we may find a better less convoluted way to do the same thing, or we may end up just not needing those fields anyway.

================
Comment at: lib/DebugInfo/PDB/Raw/ModInfo.cpp:64
@@ +63,3 @@
+  StringRef getObjectFileName() const {
+    return StringRef(getModuleName().end() + 1);
+  }
----------------
majnemer wrote:
> Incrementing an iterator past end looks suspicious.
In the general case I agree, but in this case we have two null terminated strings stored back to back in a variable length structure, so this seems like the most elegant way to express that.  Otherwise it's going to end up just being equivalent code but more verbose, since I will have to save off the module name into a temporary, then call module.data() + module.length() + 1.


http://reviews.llvm.org/D19570