This is an archive of the discontinued LLVM Phabricator instance.

[18/N] [libcxx] Use the posix code for directory_entry::__do_refresh
ClosedPublic

Authored by mstorsjo on Nov 10 2020, 8:41 AM.

Details

Summary

This works just fine for windows, as all the functions it calls are implemented and wrapped for windows.

Diff Detail

Event Timeline

mstorsjo created this revision.Nov 10 2020, 8:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 10 2020, 8:41 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
mstorsjo requested review of this revision.Nov 10 2020, 8:41 AM
mstorsjo retitled this revision from [19/N] [libcxx] Use the posix code for directory_entry::__do_refresh to [18/N] [libcxx] Use the posix code for directory_entry::__do_refresh.
amccarth accepted this revision.Dec 7 2020, 1:16 PM

LGTM.

Oh, are there tests that need to be enabled on Windows?

Oh, are there tests that need to be enabled on Windows?

There's a whole bunch of tests, and none of them are excluded on windows right now (as std::filesystem isn't possible to even build for windows as things are in git so far), so this patch reduces the amount of failing tests.

(Locally, I have a branch with a whole lot of patches on top of the testsuite for std::filesystem to disable/change bits to make it pass with MS STL, and with this patchset for the implementation in libc++, it gets to a point where the same tests that succeed with MS STL also succeed with libc++. I started out by upstreaming the least hacky bits of those patches, but there's just a whole lot of it, and it's more productive for now to focus on upstreaming the implementation. I was hoping for getting it in by the time of the llvm 12 branch, but we'll see how that goes...)

This one already has a green report from CI back when it was uploaded, and I think the surrounding area hasn't changed since, but I can push it through CI once before pushing the code, if it's otherwise ok. This is essentially only a deletion of a windows specific ifdef block, nothing more.

ldionne accepted this revision.Feb 5 2021, 11:42 AM
ldionne added a subscriber: ldionne.

I love red diffs. LGTM.

As @amccarth said - I'm relying on you running the tests on Windows locally for the correctness. Once we're trough with your patchset and filesystem builds on windows, I would be really really thankful if we could add CI on a windows machine and run the tests there.

This revision is now accepted and ready to land.Feb 5 2021, 11:42 AM