[PATCH] D40110: Use TempFile in dsymutil

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added inline comments.


================
Comment at: tools/dsymutil/dsymutil.cpp:217
 
+static std::vector<sys::fs::TempFile> TempFileStore;
 void llvm::dsymutil::exitDsymutil(int ExitStatus) {
----------------
How about wrapping this in a small RAII class? Without the need to run the interrupt handler we can get rid of `exitDsymutil`.


================
Comment at: tools/dsymutil/dsymutil.cpp:316
     // temporary files.
+    llvm::SmallString<128> TmpModel;
+    llvm::sys::path::system_temp_directory(true, TmpModel);
----------------
Can we extract the temp-file logic into a separate function that returns a `Expected<sys::fs::TempFile>`? 


================
Comment at: tools/dsymutil/dsymutil.cpp:318
+    llvm::sys::path::system_temp_directory(true, TmpModel);
+    llvm::sys::path::append(TmpModel, "dysym.tmp%%%%%.dwarf");
+
----------------
s/dysym/dsym/


================
Comment at: tools/dsymutil/dsymutil.cpp:366
       } else {
         llvm::ThreadPool Threads(NumThreads);
         Threads.async(LinkLambda);
----------------
My suggestion would require us to keep a vector of futures. Thinking about it, that also solves the problem of one thread calling `exitDsymutil` and removing temp files while another thread is still writing to them. 


https://reviews.llvm.org/D40110