[PATCH] D40057: Simplify file handling in dsymutil

[Rafael Avila de Espindola via llvm-commits]

Jonas Devlieghere via Phabricator <reviews at reviews.llvm.org> writes:

> JDevlieghere added a comment.
>
> Thanks Rafael! I added the `ToolOutputFile` when implementing the threading option because I wanted to make sure files were removed if some thread were to exist dsymutil. Because we only use threads when there are multiple architectures which in turn  implies we use temp files, we would indeed end up removing the file twice.
>
> It's definitely an improvement to keep the file handling logic out the DwarfLinker. I'm not really happy with how we deal with files in `dsymutil.cpp` though and maybe this is the opportunity to improve it. For one, I don't like that `getOutputFileName` is doing so much more than just generating a file name. I also don't like that we don't remove non-temporary output files if linking fails.
>
> Maybe we can start by splitting up the logic for temporary files (using `TempFile`) and output files (using `ToolOutputFile`). The former would contribute to your goal of unifying the use of `TempFile` and the latter would address my second concern. Things are a little tricky for dSYM bundles, where we ideally would want to remove the whole bundle rather than just the file. There's also the call to `generateUniversalBinary` which takes a file path rather than a handle.

My idea is to:
* Have all all code in DwarfLinker just take a raw_fd_ostream (this
  patch).
* Use TempFile for all temporary files including single output case.

With that the only case where temporary files would not be used is when
outputting to a special file like "-". This is similar to
FileOutputBuffer, but using a stream instead of mmap for output.

Cheers,
Rafael