[PATCH] D102736: Fix tmp files being left on Windows builds.

[Amy Huang via Phabricator via llvm-commits]

akhuang added inline comments.


================
Comment at: llvm/lib/Support/Path.cpp:1237
       RenameEC = copy_file(TmpName, Name);
       setDeleteDisposition(H, true);
     }
----------------
amccarth wrote:
> I'm curious if this path has ever been exercised.
> 
> I see that rename_handle is implemented with MoveFileExW on Windows.  MoveFileExW docs say it will fail if you're trying to move a _directory_ to another device, but that it can do copy+delete to move a _file_ to another device.  (That might require adding MOVEFILE_COPY_ALLOWED to the flags passed in the MoveFileExW in lib\Support\Windows\Path.inc near line 485.)
> 
> Anyway, it just seems like we're re-implementing functionality the OS calls can already do for us.
> 
> https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-movefileexw
 according to https://reviews.llvm.org/D50048 we used to pass MOVEFILE_COPY_ALLOWED but then removed it because it calls `rename_internal` which fails.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102736