[PATCH] D24176: Remove temoprary files.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 1 19:14:38 PDT 2016


ruiu added inline comments.

================
Comment at: COFF/DriverUtils.cpp:286
@@ +285,3 @@
+public:
+  explicit TemporaryFile(StringRef Extn) {
+    SmallString<128> S;
----------------
rnk wrote:
> Let's thread through the other parameter, so that we get "resource-NNN.obj". That was somewhat helpful for my debugging purposes, and it will show up in the debug info too.
Done.

================
Comment at: COFF/DriverUtils.cpp:300
@@ +299,3 @@
+    if (!Path.empty())
+      sys::fs::remove(Path);
+  }
----------------
rnk wrote:
> Can we assert success here?
Done.

================
Comment at: COFF/DriverUtils.cpp:584
@@ -570,3 +583,3 @@
   E.run();
-  return check(MemoryBuffer::getFile(Path), "could not open " + Path);
+  return check(MemoryBuffer::getFile(File.Path), "could not open " + File.Path);
 }
----------------
rnk wrote:
> Won't this fail on Windows, where you cannot remove a file which has an open mapping?
Surprisingly enough, it can actually deletes files. I double-checked that. Don't have a good explanation yet though.


https://reviews.llvm.org/D24176





More information about the llvm-commits mailing list