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

Fri May 21 15:27:42 PDT 2021

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