[PATCH] Add llvm-pdbdump to tools

[Chandler Carruth]

Mostly nit picking style issues. I don't know enough about COM or anything else here to do more. ;]


================
Comment at: tools/CMakeLists.txt:64
@@ -63,1 +63,3 @@
 
+if(MSVC)
+  add_llvm_tool_subdirectory(llvm-pdbdump)
----------------
Shouldn't this be checking if the host OS is Windows rather than the compiler?

================
Comment at: tools/llvm-pdbdump/COMExtras.h:18-19
@@ +17,4 @@
+
+template <class R, class... Args>
+struct function_traits<R (*)(Args...)> : public function_traits<R(Args...)> {};
+
----------------
MSVC 2012 doesn't support variadiac templates I thought?

================
Comment at: tools/llvm-pdbdump/COMExtras.h:36-40
@@ +35,7 @@
+  // template<std::size_t ArgIndex>
+  // struct Argument
+  // {
+  //   typedef typename
+  //     std::tuple_element<ArgIndex, std::tuple<Args...>>::type type;
+  // };
+  // MSVC encounters a parsing error.
----------------
Should the example code her be formatted consistently?

================
Comment at: tools/llvm-pdbdump/COMExtras.h:50-51
@@ +49,4 @@
+
+//=============================================================================
+// class ComIterator<>
+//
----------------
Use doxygen comment style instead?

================
Comment at: tools/llvm-pdbdump/COMExtras.h:63
@@ +62,3 @@
+//=============================================================================
+template <class EnumeratorType, std::size_t ArgIndex> class ComIterator {
+private:
----------------
I very much prefer that we capitalize initialisms in names properly if we're using capital letters at all. So "COMIterator" here. However, for iterator and STL-ish types we often use STL-ish naming conventions and I'd be fine with com_iterator.

================
Comment at: tools/llvm-pdbdump/llvm-pdbdump.cpp:44-47
@@ +43,6 @@
+namespace windows {
+extern std::error_code UTF8ToUTF16(StringRef utf8,
+                                   SmallVectorImpl<wchar_t> &utf16);
+extern std::error_code UTF16ToUTF8(const wchar_t *utf16, size_t utf16_len,
+                                   SmallVectorImpl<char> &utf8);
+}
----------------
If these aren't provided by Support, they should be. If they are, just include the header rather than declaring them yourself.

================
Comment at: tools/llvm-pdbdump/llvm-pdbdump.cpp:57
@@ +56,3 @@
+
+// -streams, -s
+cl::opt<bool> Streams("streams", cl::desc("Display data stream information"));
----------------
I wouldn't bother with the comment, it seems obvious from the code.

================
Comment at: tools/llvm-pdbdump/llvm-pdbdump.cpp:127
@@ +126,3 @@
+
+int main(int argc_, const char *argv_[]) {
+  // Print a stack trace if we signal out.
----------------
Why the _s?

http://reviews.llvm.org/D7153

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/