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

[Frederic Riss]

Thanks for taking the time to review that! I'll address most of your comments for the next update. Some comments inline:

================
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
 
----------------
samsonov wrote:
> I'd prefer binary name (llvm-dsymutil) to match the directory name. I'm not aware of existing policy, though.
This is in fact on purpose. The binary is currently called llvm-dsymutil so that it doesn't interfere with the system dsymutil. When the work will be complete enough to actually replace the system dsymutil, I want to rename the binary to just dsymutil so that it takes precedence when clang invokes dsymutil. I preferred to name the directory after the expected end-result rather than renaming the directory later.

================
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.");
----------------
samsonov wrote:
> Can you just use StringMap::insert() and use its return value to check if the key is already there?
I like the simplicity of operator[] more. I'm not sure that using insert() would simplify anything especially as its return value would be used only in Assert builds. 

================
Comment at: tools/dsymutil/DebugMap.cpp:31
@@ +30,3 @@
+void DebugMapObject::dump() const {
+  errs() << "Object file " << getObjectFilename() << '\n';
+  auto End = Symbols.end();
----------------
samsonov wrote:
> why not outs()? (here and below). Or, better, you can probably take
>   llvm::raw_ostream &OS
> as an input parameter.
AFAIK, dump() without argument is more canonical in LLVM, and it isn't usually even compiled in in non-debug builds.
However, you are right that in this case as I use it to emit user-visible diagnostics, this convention doesn't apply.

================
Comment at: tools/dsymutil/DebugMap.h:61
@@ +60,3 @@
+/// }
+class DebugMap
+{
----------------
samsonov wrote:
> 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.
I can rename that to DSYMDebugMap, although I find that this ties it a bit too much to Darwin. I can't be sure, but I'd expect the code using the DebugMap (as opposed to the code producing it) to be fairly generic. Not that it matters for the current unique useless though.

================
Comment at: tools/dsymutil/MachODebugMapParser.cpp:18
@@ +17,3 @@
+
+static ErrorOr<OwningBinary<MachOObjectFile>> createMachOBinary(StringRef file) {
+  ErrorOr<OwningBinary<Binary>> BinaryOrErr = createBinary(file);
----------------
samsonov wrote:
> Can you make use of ObjectFile::createMachOObjectFile() here?
I think the quantity of code would be the same because to use it I have to handle the MemoryBuffer creation myself. I'm not sure which one is preferable.

================
Comment at: tools/dsymutil/MachODebugMapParser.h:53
@@ +52,3 @@
+
+  void newDebugMapObject(StringRef Filename);
+  void resetParserState();
----------------
samsonov wrote:
> newDebugMapObject() doesn't actually return anything, which is slightly confusing, especially when you call
>   return newDebugMapObject()
> below.
Yeah the name dates back to a version where it would actually return the allocated object. I'll change that to switchToNewDebugMapObject(Name). Do you have an issue with the return voidReturningFunc(); idiom? I kinda like it when it allows to do early exits without having to brace the if body.

http://reviews.llvm.org/D6242