[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

319–320

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>.

36

Please add an empty line here.

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

1

@ldionne Can we just remove this file?

[Mordante]

libcxx/docs/Status/Cxx17Issues.csv

319–320

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 ↗

(On Diff #430045)

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

 "`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|",""
 No newline at end of file

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> {
+
+  _LIBCPP_INLINE_VISIBILITY
+  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/include/filesystem

 template <>
 inline constexpr bool std::ranges::enable_borrowed_range<std::filesystem::recursive_directory_iterator> = true;
 
 template <>
 inline constexpr bool std::ranges::enable_view<std::filesystem::directory_iterator> = true;
 template <>
 inline constexpr bool std::ranges::enable_view<std::filesystem::recursive_directory_iterator> = true;
 
+template <>
+struct std::hash<std::filesystem::path>;
+
 */
 
 #include <__assert> // all public C++ headers provide the assertion handler
 #include <__config>
 #include <__filesystem/copy_options.h>
 #include <__filesystem/directory_entry.h>
 #include <__filesystem/directory_iterator.h>
 #include <__filesystem/directory_options.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"
const int E = TC.expect;
       ASSERT_NOEXCEPT(p1 <= p2);
       ASSERT_NOEXCEPT(p1 >  p2);
       ASSERT_NOEXCEPT(p1 >= p2);
     }
     { // check hash values
       auto h1 = hash_value(p1);
       auto h2 = hash_value(p2);
       assert((h1 == h2) == (p1 == p2));
+      assert(h1 == std::hash<std::filesystem::path>()(p1));
+      assert(h2 == std::hash<std::filesystem::path>()(p2));
       // check signature
       ASSERT_SAME_TYPE(size_t, decltype(hash_value(p1)));
       ASSERT_NOEXCEPT(hash_value(p1));
+      ASSERT_SAME_TYPE(size_t, decltype(std::hash<std::filesystem::path>()(p1)));
+      ASSERT_NOEXCEPT(std::hash<std::filesystem::path>()(p1));
     }
   }
 }
 
 int CompareElements(std::vector<std::string> const& LHS, std::vector<std::string> const& RHS) {
   bool IsLess = std::lexicographical_compare(LHS.begin(), LHS.end(), RHS.begin(), RHS.end());
   if (IsLess)
     return -1;

libcxx/test/std/input.output/filesystems/class.path/path.nonmember/enabled_hash.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
 
-// The "hash_value" function is tested as part of [path.compare]
-// in class.path/path.members/path.compare.pass.cpp
+// <filesystem>
+
+// Test that <filesystem> provides all of the arithmetic, enum, and pointer
+// hash specializations.
+
+#include <filesystem>
+
+#include "poisoned_hash_helper.h"
+
+#include "test_macros.h"
+
 int main(int, char**) {
+  test_library_hash_specializations_available();
+  { test_hash_enabled_for_type<std::filesystem::path>(); }
+
   return 0;
 }

libcxx/test/std/input.output/filesystems/class.path/path.nonmember/hash_tested_elswhere.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
 
-// The "hash_value" function is tested as part of [path.compare]
-// in class.path/path.members/path.compare.pass.cpp
-int main(int, char**) {
-  return 0;
-}
+// The "hash_value" function and std::hash specialization are
+// tested as part of [path.compare] in
+// class.path/path.members/path.compare.pass.cpp
+
+int main(int, char**) { return 0; }