[PATCH] D20936: [pdb] Parse module line info

[Zachary Turner via llvm-commits]

zturner marked 3 inline comments as done.
zturner added a comment.

I'll address the remainder of the comments in a followup


================
Comment at: include/llvm/DebugInfo/CodeView/ModuleSubstream.h:41-42
@@ +40,4 @@
+  support::ulittle32_t BlockSize; // Code size of block, in bytes.
+  // The following two variable length arrays appear immediately after the
+  // header.  The structure definitions follow.
+  // LineNumberEntry   Lines[NumLines];
----------------
Added comments

================
Comment at: include/llvm/DebugInfo/CodeView/ModuleSubstreamVisitor.h:23
@@ +22,3 @@
+
+struct LineColumnEntry {
+  support::ulittle32_t Offset;
----------------
This doesn't correspond to any CodeView types.  It's just the ValueType of the array.  i.e. when you iterate the array you will be iterating over a sequence of these.  The value type can be whatever you want (just depends how you write the extractor), so here I chose to make it something that provides access to the 2 underlying embedded arrays and the offset field.

================
Comment at: include/llvm/DebugInfo/CodeView/StreamReader.h:56
@@ -57,1 +55,3 @@
+  template <typename T, typename U>
+  Error readArray(VarStreamArray<T, U> &Array, uint32_t Size) {
     StreamRef S;
----------------
Because you don't ever have to think about what to pass for it, and it's not used in the function except for passthrough.  You're never explicitly parameterizing the function, it's always deduced based on the type of the `VarStreamArray` you pass in.  

================
Comment at: lib/DebugInfo/CodeView/ModuleSubstream.cpp:12
@@ +11,3 @@
+
+#include "llvm/DebugInfo/CodeView/StreamReader.h"
+
----------------
Don't we usually put a blank line between the TU's "primary" header and the rest of the headers?  I always do, wonder if I've been doing it wrong.


http://reviews.llvm.org/D20936