[libc++] Implement LWG3657 (std::hash<std::filesystem::path> is not enabled)

[ldionne]

Summary

This patch implements LWG-3657 (std::hash<std::filesystem::path> is not enabled) in libcxx. Since the February 2022 was virtual, I don't know what location to specify in docs/Status.

[philnik]

Could you rename the PR to something along the lines of [libc++] Implement LWG3657 to make clear which project this is part of and that you are implementing LWG3657?

libcxx/docs/Status/Cxx17Issues.csv

318–319

This is currently in Cxx2bIssues.csv. I don't know if it should be there or here. @ldionne
In the last "" should be the version in which it was implemented (15.0) in this case.

libcxx/include/__filesystem/path.h

1022

You don't need availability for this function. It's not part of the dylib.

1027

Please don't inherit from unary_function. This should be removed since C++17.

1029

Please use _LIBCPP_HIDE_FROM_ABI.

1030

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

12

This doesn't test <functional>.

34–36

Please add an empty line here.

libcxx/test/std/input.output/filesystems/class.path/path.nonmember/hash_tested_elswhere.pass.cpp

1 ↗

(On Diff #428679)

@ldionne Can we just remove this file?

[Mordante]

libcxx/docs/Status/Cxx17Issues.csv

318–319

In general LWG issues are listed in the status list C++ version it was voted in the WP.
So if this was voted in the February 2202 virtual plenary it should be in Cxx2bIssues
We still retroactively apply the fix to older versions of the Standard.

[Fulgen]

Fixes according to review feedback

[var-const]

@Fulgen Fly-by comment: can you please change the title (i.e., the first line) of the patch to include the name of the paper as well? It would make it a little easier to understand what the patch is about for somebody looking through the list of Phabricator reviews (or the output of git log --oneline).

[Fulgen]

Did that now, assuming you meant retitling the revision (there are now three commits in this revision due to feedback fixes, so I don't think you mean the commit message).

[philnik]

It looks like you forgot to add the first commit to you diff.

[Fulgen]

Add missing commit

[Fulgen]

• Readd accidentally deleted newline at end of file

[Fulgen]

Messed up updating the revision with arc, fixed now.

[Fulgen]

Is there anything still missing? As far as I can tell, I've implemented the feedback.

[philnik]

In D125394#3952100, @Fulgen wrote:

Is there anything still missing? As far as I can tell, I've implemented the feedback.

Please fix the CI and mark completed comments as Done. (BTW, next time you can ping earlier than 6 months later if you want :-) )

[Mordante]

In D125394#3952128, @philnik wrote:

In D125394#3952100, @Fulgen wrote:

Is there anything still missing? As far as I can tell, I've implemented the feedback.

Please fix the CI and mark completed comments as Done. (BTW, next time you can ping earlier than 6 months later if you want :-) )

Unfortunately sometimes we miss message, so pinging after a week would be better.

Please make sure you rebase your patch on the latest main before updating it.

libcxx/docs/Status/Cxx2bIssues.csv

158

This should be 16 now.

[ldionne]

Aw, sorry for not noticing this earlier. Phabricator notifications are kinda broken, they don't allow direct mentions to be separated from the rest of all reviews, which means I didn't see the pings here.

This was done and committed by D143452.. I'm sorry for the duplicate work, but it seems like we can close this now :-/

[ldionne]

Closing since this was done in D143452.

libcxx/docs/Status/Cxx17Issues.csv

 "`2905 <https://wg21.link/LWG2905>`__","is_constructible_v<unique_ptr<P, D>, P, D const &> should be false when D is not copy constructible","Kona","|Complete|",""
 "`2908 <https://wg21.link/LWG2908>`__","The less-than operator for shared pointers could do more","Kona","|Complete|",""
 "`2911 <https://wg21.link/LWG2911>`__","An is_aggregate type trait is needed","Kona","|Complete|",""
 "`2921 <https://wg21.link/LWG2921>`__","packaged_task and type-erased allocators","Kona","|Complete|",""
 "`2934 <https://wg21.link/LWG2934>`__","optional<const T> doesn't compare with T","Kona","|Complete|",""
 "","","","",""
 "`2901 <https://wg21.link/LWG2901>`__","Variants cannot properly support allocators","Toronto","|Complete|",""
 "`2955 <https://wg21.link/LWG2955>`__","``to_chars / from_chars``\  depend on ``std::string``\ ","Toronto","Resolved by `P0682R1 <https://wg21.link/P0682R1>`__",""
-"`2956 <https://wg21.link/LWG2956>`__","``filesystem::canonical()``\  still defined in terms of ``absolute(p, base)``\ ","Toronto","|Complete|",""
-"","","","",""
-"`3657 <https://wg21.link/LWG3657>`__","``std::hash<std::filesystem::path>`` is not enabled","","|Complete|",""
+"`2956 <https://wg21.link/LWG2956>`__","``filesystem::canonical()``\  still defined in terms of ``absolute(p, base)``\ ","Toronto","|Complete|",""
 

libcxx/docs/Status/Cxx2bIssues.csv

 "`3619 <https://wg21.link/LWG3619>`__","Specification of ``vformat_to`` contains ill-formed ``formatted_size`` calls","February 2022","|Nothing to do|","","|format|"
 "`3621 <https://wg21.link/LWG3621>`__","Remove feature-test macro ``__cpp_lib_monadic_optional`` ","February 2022","",""
 "`3632 <https://wg21.link/LWG3632>`__","``unique_ptr`` ""Mandates: This constructor is not selected by class template argument deduction""","February 2022","|Nothing to do|",""
 "`3643 <https://wg21.link/LWG3643>`__","Missing ``constexpr`` in ``std::counted_iterator`` ","February 2022","",""
 "`3648 <https://wg21.link/LWG3648>`__","``format`` should not print ``bool`` with ``'c'`` ","February 2022","|Complete|","15.0","|format|"
 "`3649 <https://wg21.link/LWG3649>`__","[fund.ts.v2] Reinstate and bump ``__cpp_lib_experimental_memory_resource`` feature test macro","February 2022","",""
 "`3650 <https://wg21.link/LWG3650>`__","Are ``std::basic_string`` 's ``iterator`` and ``const_iterator`` constexpr iterators?","February 2022","|Nothing to do|",""
 "`3654 <https://wg21.link/LWG3654>`__","``basic_format_context::arg(size_t)`` should be ``noexcept`` ","February 2022","|Complete|","15.0","|format|"
-"`3657 <https://wg21.link/LWG3657>`__","``std::hash<std::filesystem::path>`` is not enabled","February 2022","",""
+"`3657 <https://wg21.link/LWG3657>`__","``std::hash<std::filesystem::path>`` is not enabled","February 2022","|Complete|","15.0","|filesystem|"
 "`3660 <https://wg21.link/LWG3660>`__","``iterator_traits<common_iterator>::pointer`` should conform to §[iterator.traits]","February 2022","|Complete|","14.0"
 "`3661 <https://wg21.link/LWG3661>`__","``constinit atomic<shared_ptr<T>> a(nullptr);`` should work","February 2022","",""
 "","","","",""
 "`3645 <https://wg21.link/LWG3645>`__","``resize_and_overwrite`` is overspecified to call its callback with lvalues","Not voted in","|Complete|","14.0",""
 "`3656 <https://wg21.link/LWG3656>`__","Inconsistent bit operations returning a count","Not voted in","|Complete|","15.0",""
 "","","","",""

libcxx/include/__filesystem/path.h

 
 _LIBCPP_FUNC_VIS
 size_t hash_value(const path& __p) noexcept;
 
 _LIBCPP_AVAILABILITY_FILESYSTEM_POP
 
 _LIBCPP_END_NAMESPACE_FILESYSTEM
 
-_LIBCPP_AVAILABILITY_FILESYSTEM_PUSH
-
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <>
-struct _LIBCPP_TEMPLATE_VIS hash<_VSTD_FS ::path> : public unary_function<_VSTD_FS ::path, size_t> {
+struct _LIBCPP_TEMPLATE_VIS hash<_VSTD_FS ::path> {
 
-  _LIBCPP_INLINE_VISIBILITY
-  size_t operator()(const _VSTD_FS ::path& __val) const _NOEXCEPT { return _VSTD_FS ::hash_value(__val); }
+  _LIBCPP_HIDE_FROM_ABI
+  size_t operator()(const _VSTD_FS ::path& __val) const noexcept { return _VSTD_FS ::hash_value(__val); }
 };
 
 _LIBCPP_END_NAMESPACE_STD
 
-_LIBCPP_AVAILABILITY_FILESYSTEM_POP
-
 #endif // _LIBCPP_CXX03_LANG
 
 #endif // _LIBCPP___FILESYSTEM_PATH_H

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

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03
 
 // <filesystem>
-// <functional>
 
 // class path
 
 // int compare(path const&) const noexcept;
 // int compare(string_type const&) const;
 // int compare(value_type const*) const;
 //
 // bool operator==(path const&, path const&) noexcept;
 // bool operator!=(path const&, path const&) noexcept;
 // bool operator< (path const&, path const&) noexcept;
 // bool operator<=(path const&, path const&) noexcept;
 // bool operator> (path const&, path const&) noexcept;
 // bool operator>=(path const&, path const&) noexcept;
 //
 // size_t hash_value(path const&) noexcept;
 //
 // template <class T>
 // struct hash
-//     : public unary_function<T, size_t>
 // {
 //     size_t operator()(T val) const;
 // };
 
 #include "filesystem_include.h"
+
 #include <functional>
 #include <type_traits>
 #include <vector>
 #include <cassert>
 
 #include "test_macros.h"
 #include "test_iterators.h"
 #include "count_new.h"
 #include "filesystem_test_helper.h"