[PATCH] D111875: [Support] [Windows] Manually clean up temp files on network shares

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 15 06:17:12 PDT 2021


mstorsjo added inline comments.


================
Comment at: llvm/lib/Support/Path.cpp:1212
+  std::error_code RemoveEC;
+  if (DeleteOnClose && !TmpName.empty()) {
+    RemoveEC = fs::remove(TmpName);
----------------
jhenderson wrote:
> I'm not really liking the duplicate code between this and the below non-Windows block. Is there a way we could avoid this duplication?
Sure, it should be pretty straightforward to make it one generic codepath with a couple smaller platform specific ifdefs instead.


================
Comment at: llvm/lib/Support/Path.cpp:1249-1252
+      Delete = true;
+      setDeleteDisposition(H, Delete);
+      if (!Delete)
+        DeleteOnClose = true;
----------------
jhenderson wrote:
> Similar comment to above: this code is duplicated a few lines down. Perhaps another function, or maybe even mode the code into setDeleteDisposition somehow?
Moving it into `setDeleteDisposition` is hard as that one isn't a member of `TempFile`, but I can try to factorize the duplication at least.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111875/new/

https://reviews.llvm.org/D111875



More information about the llvm-commits mailing list