[PATCH] D19899: [codeview] Add a type visitor to help abstract away type stream handling

[Zachary Turner via llvm-commits]

zturner added inline comments.

================
Comment at: include/llvm/DebugInfo/CodeView/CVTypeVisitor.h:30-36
@@ +29,9 @@
+  bool consumeObject(ArrayRef<uint8_t> &Data, const T *&Res) {
+    if (Data.size() < sizeof(*Res)) {
+      HadError = true;
+      return false;
+    }
+    Res = reinterpret_cast<const T *>(Data.data());
+    Data = Data.drop_front(sizeof(*Res));
+    return true;
+  }
----------------
In the future we could get rid of this entirely and use the `StreamReader` interface in `DebugInfoPDB`.  It wouldn't work right now and some refactoring would have to happen, but I think the design of it is flexible enough to support this in theory.

================
Comment at: include/llvm/DebugInfo/CodeView/CVTypeVisitor.h:69
@@ +68,3 @@
+        break;
+#define TYPE_RECORD(ClassName, LeafEnum)                                       \
+  case LeafEnum: {                                                             \
----------------
How about moving this `#define` before the switch statement so it doesn't mess up the indentation?

================
Comment at: include/llvm/DebugInfo/CodeView/TypeRecords.def:20-22
@@ +19,5 @@
+
+#ifndef TYPE_RECORD_ALIAS
+#define TYPE_RECORD_ALIAS(ClassName, LeafEnum) TYPE_RECORD(ClassName, LeafEnum)
+#endif
+
----------------
Why do we need these `_ALIAS` macros?  They seem to do nothing, why not just use `TYPE_RECORD()` everywhere?


http://reviews.llvm.org/D19899