[PATCH] D20945: pdbdump: print out TPI hashes

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 2 20:51:29 PDT 2016


zturner 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());
   }
----------------
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.)

================
Comment at: include/llvm/DebugInfo/PDB/Raw/TpiStream.h:33
@@ -31,1 +32,3 @@
 public:
+  // Corresponds to `OffCb`.
+  struct TypeIndexOffset {
----------------
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.

================
Comment at: include/llvm/DebugInfo/PDB/Raw/TpiStream.h:72
@@ -58,1 +71,3 @@
+
+raw_ostream &operator<<(raw_ostream &OS, const TpiStream::TypeIndexOffset &);
 }
----------------
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`


http://reviews.llvm.org/D20945





More information about the llvm-commits mailing list