[PATCH] D40057: Simplify file handling in dsymutil

[Jonas Devlieghere via llvm-commits]

> On 15 Nov 2017, at 16:36, Rafael Avila de Espindola <rafael.espindola at gmail.com> wrote:
> 
> 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.

Alright, seems like you plan on doing at least part of what I have in mind. We can discuss this further in the next differential which will be more relevant.  

> 
> 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