[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