[PATCH] Initial commit for the llvm-dsymutil tool.

[Alexey Samsonov]

A few drive-by comments.

================
Comment at: tools/Makefile:36
@@ -35,3 +35,3 @@
                  llvm-profdata llvm-symbolizer obj2yaml yaml2obj llvm-c-test \
-                 llvm-vtabledump verify-uselistorder
+                 llvm-vtabledump verify-uselistorder dsymutil
 
----------------
I'd prefer binary name (llvm-dsymutil) to match the directory name. I'm not aware of existing policy, though.

================
Comment at: tools/dsymutil/DebugMap.cpp:24
@@ +23,3 @@
+  SymbolMapping &Sym = Symbols[Name];
+  assert(Sym.ObjectAddress == 0 && Sym.BinaryAddress == 0 &&
+         "Symbol added twice in the same file.");
----------------
Can you just use StringMap::insert() and use its return value to check if the key is already there?

================
Comment at: tools/dsymutil/DebugMap.cpp:31
@@ +30,3 @@
+void DebugMapObject::dump() const {
+  errs() << "Object file " << getObjectFilename() << '\n';
+  auto End = Symbols.end();
----------------
why not outs()? (here and below). Or, better, you can probably take
  llvm::raw_ostream &OS
as an input parameter.

================
Comment at: tools/dsymutil/DebugMap.cpp:55
@@ +54,3 @@
+void DebugMap::dump() const {
+  errs() << "DEBUG MAP:\n";
+  for (const auto &Obj: objects())
----------------
see note about llvm::raw_ostream above.

================
Comment at: tools/dsymutil/DebugMap.h:61
@@ +60,3 @@
+/// }
+class DebugMap
+{
----------------
llvm::DebugMap is too generic, and I can imagine it can potentially conflict with some other class. Consider introducing a namespace for dsymutil-specific classes.

================
Comment at: tools/dsymutil/DwarfLinker.cpp:14
@@ +13,3 @@
+
+DwarfLinker::DwarfLinker(StringRef Filename)
+  : OutputFilename(Filename)
----------------
s/Filename/OutputFilename (to match ctor declaration).

================
Comment at: tools/dsymutil/MachODebugMapParser.cpp:18
@@ +17,3 @@
+
+static ErrorOr<OwningBinary<MachOObjectFile>> createMachOBinary(StringRef file) {
+  ErrorOr<OwningBinary<Binary>> BinaryOrErr = createBinary(file);
----------------
Can you make use of ObjectFile::createMachOObjectFile() here?

================
Comment at: tools/dsymutil/MachODebugMapParser.cpp:52
@@ +51,3 @@
+            + Error.message() + "\n");
+    CurrentDebugMapObject = nullptr;
+    return;
----------------
This line is not needed, you already call resetParserState() above.

================
Comment at: tools/dsymutil/MachODebugMapParser.h:53
@@ +52,3 @@
+
+  void newDebugMapObject(StringRef Filename);
+  void resetParserState();
----------------
newDebugMapObject() doesn't actually return anything, which is slightly confusing, especially when you call
  return newDebugMapObject()
below.

http://reviews.llvm.org/D6242