[PATCH] D20945: pdbdump: print out TPI hashes

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: include/llvm/DebugInfo/CodeView/StreamArray.h:188
@@ -187,3 +187,3 @@
   FixedStreamArrayIterator<T> end() const {
-    return FixedStreamArrayIterator<T>(*this);
+    return FixedStreamArrayIterator<T>(*this, size());
   }
----------------
zturner wrote:
> I actually fixed this same bug in a patch that went in.  So I guess you will have a merge conflict.  I did it a little bit differently, but I guess both ways work.  When you do merge, feel free to keep whichever you prefer.  This way might be a little faster since it drops 2 conditionals from the ++ operator (although it has the downside that if you increment an iterator that is already at end, it will no longer compare equal to end.  Not sure if we care, since incrementing end is already undefined behavior.)
I'm not an C++ expert, but I think in the first place we shouldn't increment an iterator that is already at end(). At least I believe if an iterator is a pointer pointing to a plain C array, the result of incrementing an iterator at end() is undefined.

================
Comment at: include/llvm/DebugInfo/PDB/Raw/TpiStream.h:33
@@ -31,1 +32,3 @@
 public:
+  // Corresponds to `OffCb`.
+  struct TypeIndexOffset {
----------------
zturner wrote:
> the first member of `OffCb` isn't necessarily a type index though, so I don't think this comment is entirely accurate.  I'm guessing that in the Microsoft code they are using an `OffCb` structure parsing this hash table, but I guess that's just because the `Offset` field coincidentally has the same type as a type index.  I think we should probably just delete the comment.  Also, I've been putting structs like this in `RawTypes.h` so that they can be re-used from other code when we need them.
Will do.

================
Comment at: include/llvm/DebugInfo/PDB/Raw/TpiStream.h:72
@@ -58,1 +71,3 @@
+
+raw_ostream &operator<<(raw_ostream &OS, const TpiStream::TypeIndexOffset &);
 }
----------------
zturner wrote:
> I was doing this at first too, but I think some people had objections to overloading the `<<` operator.  I guess you should make a `printTypeIndexOffset()` function in `llvm-pdbdump.cpp`
Will do.


http://reviews.llvm.org/D20945