This is an archive of the discontinued LLVM Phabricator instance.

Support: On Windows, use CreateFileW to delete files in sys::fs::remove().
ClosedPublic

Authored by pcc on Oct 9 2017, 9:35 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

pcc created this revision.Oct 9 2017, 9:35 PM
pcc updated this revision to Diff 118428.Oct 10 2017, 10:54 AM
  • Use CreateFile
pcc retitled this revision from Support: On Windows, speculatively call DeleteFileW from sys::fs::remove(). to Support: On Windows, use CreateFileW to delete files in sys::fs::remove()..Oct 10 2017, 10:54 AM
zturner accepted this revision.Oct 10 2017, 11:26 AM

Can you add a comment indicating what the first two flags are used for? They aren't very common, so it might help someone in the future understand what's going on.

Also maybe a top-level comment indicating why we're doing things this way instead of the old way.

Don't need to re-upload, lgtm already.

This revision is now accepted and ready to land.Oct 10 2017, 11:26 AM
amccarth added inline comments.
llvm/lib/Support/Windows/Path.inc
265 ↗(On Diff #118428)

Zach's right that this is likely a tad faster, but it's also a little less clear. How about a comment to make it point out that we're using CreateFileW to mark the file/direction for deletion?

This revision was automatically updated to reflect the committed changes.
pcc marked an inline comment as done.
pcc added a comment.Oct 10 2017, 12:40 PM

Thanks, I've added a big comment block explaining what's going on.