This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Implement LWG3657 std::hash<filesystem::path>
ClosedPublic

Authored by ldionne on Feb 6 2023, 5:55 PM.

Details

Reviewers
philnik
Group Reviewers
Restricted Project
Commits
rG1cf344d9465a: [libc++] Implement LWG3657 std::hash<filesystem::path>
Summary

This is implemented as a DR on top of C++17.

Diff Detail

Event Timeline

ldionne created this revision.Feb 6 2023, 5:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 6 2023, 5:55 PM
ldionne requested review of this revision.Feb 6 2023, 5:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 6 2023, 5:55 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
jloser added a subscriber: jloser.Feb 6 2023, 7:27 PM

Seems reasonable to me. We should close https://reviews.llvm.org/D125394 when/if we land your review here.

ldionne updated this revision to Diff 495362.Feb 6 2023, 8:09 PM

Rebase onto main

ldionne updated this revision to Diff 495369.Feb 6 2023, 9:05 PM

Fix namespace qualification.

philnik accepted this revision.Feb 7 2023, 2:49 AM
philnik added a subscriber: philnik.

LGTM % nits.

libcxx/include/__filesystem/path.h
1094

I don't think we have to inherit from unary_function here, and I don't really see a point if we don't have to.

libcxx/test/std/input.output/filesystems/class.path/path.member/path.compare.pass.cpp
138–139

Unfortunatley, we provide std::filesystem as an extension in C++11 and C++14 under std::__fs::filesystem.

This revision is now accepted and ready to land.Feb 7 2023, 2:49 AM
ldionne marked an inline comment as done.Feb 7 2023, 7:40 AM
ldionne updated this revision to Diff 495536.Feb 7 2023, 7:40 AM

Fix test in C++11/C++14

ldionne marked an inline comment as done.Feb 7 2023, 7:45 AM
ldionne added inline comments.
libcxx/include/__filesystem/path.h
1094

The std::hash base template is specified to have those members in C++17 even though they are deprecated. So to be strictly conforming, I think it's better to provide them.

ldionne updated this revision to Diff 495689.Feb 7 2023, 5:11 PM
ldionne marked an inline comment as done.

Fix backdeployment issue

This revision was landed with ongoing or failed builds.Feb 7 2023, 10:06 PM
This revision was automatically updated to reflect the committed changes.