[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