[PATCH] llvm-vtabledump: A vtable dumper

[Hans Wennborg]

================
Comment at: tools/llvm-vtabledump/Error.cpp:20
@@ +19,3 @@
+namespace {
+class _vtabledump_error_category : public std::error_category {
+public:
----------------
Why the leading underscore?

================
Comment at: tools/llvm-vtabledump/Error.cpp:27
@@ +26,3 @@
+
+const char *_vtabledump_error_category::name() const {
+  return "llvm.vtabledump";
----------------
How about just defining this and message() in the class?

================
Comment at: tools/llvm-vtabledump/Error.h:36
@@ +35,3 @@
+template <>
+struct is_error_code_enum<llvm::vtabledump_error> : std::true_type {};
+}
----------------
Is this OK because we're not adding anything to namespace std, just specializing an already existing template in there?

================
Comment at: tools/llvm-vtabledump/llvm-vtabledump.cpp:58
@@ +57,3 @@
+
+static void reportError(StringRef Input, std::error_code EC) {
+  if (Input == "-")
----------------
Could this just call reportError(Input, EC.message()) ?

================
Comment at: tools/llvm-vtabledump/llvm-vtabledump.cpp:71
@@ +70,3 @@
+
+  errs() << Input << ": " << Message << "\n";
+  ReturnValue = EXIT_FAILURE;
----------------
Should this flush like the other reportError does?

================
Comment at: tools/llvm-vtabledump/llvm-vtabledump.cpp:82
@@ +81,3 @@
+      return;
+    if (SymName.startswith("??_7")) {
+      object::section_iterator SecI(Obj->section_begin());
----------------
Maybe a comment about what ??_7 signifies?

Maybe the stuff in this if branch can be broken out to a helper function?

(These comments apply to the ??_8 side below too.)

================
Comment at: tools/llvm-vtabledump/llvm-vtabledump.cpp:135
@@ +134,3 @@
+
+/// @brief Dumps each vtable file in \a Arc;
+static void dumpArchive(const Archive *Arc) {
----------------
IIRC, \brief isn't necessary on one-line doxygen comments. Applies below too.

================
Comment at: tools/llvm-vtabledump/llvm-vtabledump.h:1
@@ +1,2 @@
+//===-- llvm-vtabledump.h -------------------------------------------------===//
+//
----------------
I thought we usually put "C++" in these things, but not in the tools/ dir?

================
Comment at: tools/llvm-vtabledump/llvm-vtabledump.h:10
@@ +9,3 @@
+
+#ifndef LLVM_TOOLS_READ_OBJ_H
+#define LLVM_TOOLS_READ_OBJ_H
----------------
Wrong include guard name.

http://reviews.llvm.org/D4584