[PATCH] D24176: Remove temoprary files.
Adrian McCarthy via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 2 09:41:20 PDT 2016
amccarth added a subscriber: amccarth.
amccarth added a comment.
I know that in theory, TemporaryFile is very simple and unlikely to fail. In practice, though, Windows can be very finicky about deleting files, so it might be nice to add a test for TemporaryFile. I'm surprised LLVM doesn't already have such a class that's already well-tested.
================
Comment at: COFF/DriverUtils.cpp:286
@@ +285,3 @@
+public:
+ explicit TemporaryFile(StringRef Prefix, StringRef Extn) {
+ SmallString<128> S;
----------------
I don't think the `explicit` does anything here, since the constructor requires two parameters.
================
Comment at: COFF/DriverUtils.cpp:300
@@ +299,3 @@
+ return;
+ sys::fs::remove(Path);
+ assert(!sys::fs::exists(Path));
----------------
I'm concerned the assertion could fail sporadically on Windows, since there's a delay between asking for the file to be deleted and the actual deletion of the file.
https://reviews.llvm.org/D24176
More information about the llvm-commits
mailing list