[PATCH] D20534: Make a symbol visitor and use it to dump CV symbol records

[Reid Kleckner via llvm-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


================
Comment at: include/llvm/DebugInfo/CodeView/CVSymbolVisitor.h:24
@@ +23,3 @@
+public:
+  CVSymbolVisitor(SymbolDumpDelegate *ObjDelegate) : ObjDelegate(ObjDelegate) {}
+
----------------
Let's not have the visitor / deserializer know about dumping. Can you make a base class like `VisitorDelegate` that doesn't have any of the dumping related methods in it?

================
Comment at: include/llvm/DebugInfo/CodeView/CVSymbolVisitor.h:42-43
@@ +41,4 @@
+/// expected to consume the trailing bytes used by the field.
+/// FIXME: Make the visitor interpret the trailing bytes so that clients don't
+/// need to.
+#define SYMBOL_RECORD(EnumName, EnumVal, Name)                                 \
----------------
This FIXME is fixed here and in the type visitor where it came from. You fixed it. :)

================
Comment at: include/llvm/DebugInfo/CodeView/CVSymbolVisitor.h:89-90
@@ +88,4 @@
+  /// including the fixed-length record prefix.
+  void visitSymbolBegin(SymbolKind Leaf, ArrayRef<uint8_t> RecordData) {}
+  void visitSymbolEnd(SymbolKind Leaf, ArrayRef<uint8_t> OriginalSymData) {}
+
----------------
These parameters can be named the same thing.


http://reviews.llvm.org/D20534