Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc++][modules] Refactor poisoned_hash_helper #108296

Merged

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Sep 11, 2024

The poisoned_hash_helper header was relying on an implicit forward declaration of std::hash located in <type_traits>. When we improve the modularization of the library, that causes issues, in addition to being a fundamentally non-portable assumption in the test suite.

It turns out that the reason for relying on a forward declaration is to be able to test that std::hash is not provided if we don't include any header that provides it. But testing that is actually both non-portable and not really useful.

Indeed, what harm does it make if additional headers provide std::hash specializations? That would certainly be conforming -- the Standard never requires an implementation to avoid providing a declaration when a given header is included, instead it mandates what must be provided for sure. In that spirit, it would be conforming for e.g. <cstddef> to define the hash specializations if that was our desire. I also don't read https://wg21.link/P0513R0 as going against that statement. Hence, this patch just removes that test which doesn't carry its weight.

Fixes #56938

The poisoned_hash_helper header was relying on an implicit forward
declaration of std::hash located in <type_traits>. When we improve
the modularization of the library, that causes issues, in addition
to being a fundamentally non-portable assumption in the test suite.

It turns out that the reason for relying on a forward declaration
is to be able to test that std::hash is *not* provided if we don't
include any header that provides it. But testing that is actually
both non-portable and not really useful.

Indeed, what harm does it make if additional headers provide std::hash
specializations? That would certainly be conforming -- the Standard
never requires an implementation to avoid providing a declaration when
a given header is included, instead it mandates what *must* be provided
for sure. In that spirit, it would be conforming for e.g. `<cstddef>`
to define the hash specializations if that was our desire. I also don't
read https://wg21.link/P0513R0 as going against that statement. Hence,
this patch just removes that test which doesn't carry its weight.

Fixes llvm#56938
@ldionne ldionne requested a review from a team as a code owner September 11, 2024 21:34
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 11, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

The poisoned_hash_helper header was relying on an implicit forward declaration of std::hash located in <type_traits>. When we improve the modularization of the library, that causes issues, in addition to being a fundamentally non-portable assumption in the test suite.

It turns out that the reason for relying on a forward declaration is to be able to test that std::hash is not provided if we don't include any header that provides it. But testing that is actually both non-portable and not really useful.

Indeed, what harm does it make if additional headers provide std::hash specializations? That would certainly be conforming -- the Standard never requires an implementation to avoid providing a declaration when a given header is included, instead it mandates what must be provided for sure. In that spirit, it would be conforming for e.g. &lt;cstddef&gt; to define the hash specializations if that was our desire. I also don't read https://wg21.link/P0513R0 as going against that statement. Hence, this patch just removes that test which doesn't carry its weight.

Fixes #56938


Patch is 22.09 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/108296.diff

16 Files Affected:

  • (modified) libcxx/include/type_traits (-1)
  • (modified) libcxx/test/std/containers/sequences/vector.bool/enabled_hash.pass.cpp (+2-2)
  • (modified) libcxx/test/std/diagnostics/syserr/syserr.hash/enabled_hash.pass.cpp (+2-2)
  • (modified) libcxx/test/std/experimental/memory/memory.observer.ptr/hash.pass.cpp (+1-1)
  • (modified) libcxx/test/std/input.output/filesystems/class.path/path.member/path.hash_enabled.pass.cpp (+1-1)
  • (modified) libcxx/test/std/strings/basic.string.hash/enabled_hashes.pass.cpp (+8-8)
  • (modified) libcxx/test/std/strings/string.view/string.view.hash/enabled_hashes.pass.cpp (+7-7)
  • (modified) libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/enabled_hashes.pass.cpp (+1-1)
  • (modified) libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.hash/hash_shared_ptr.pass.cpp (+2-2)
  • (modified) libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.hash/hash_unique_ptr.pass.cpp (+4-4)
  • (modified) libcxx/test/std/utilities/optional/optional.hash/hash.pass.cpp (+8-8)
  • (modified) libcxx/test/std/utilities/template.bitset/bitset.hash/enabled_hash.pass.cpp (+4-4)
  • (modified) libcxx/test/std/utilities/type.index/type.index.synopsis/hash_type_index.pass.cpp (+1-1)
  • (modified) libcxx/test/std/utilities/variant/variant.hash/hash.pass.cpp (+7-7)
  • (modified) libcxx/test/support/poisoned_hash_helper.h (+71-142)
  • (removed) libcxx/test/support/test.support/test_poisoned_hash_helper.pass.cpp (-33)
diff --git a/libcxx/include/type_traits b/libcxx/include/type_traits
index 5937d4fdc9e1a7..26c85f2284e2fd 100644
--- a/libcxx/include/type_traits
+++ b/libcxx/include/type_traits
@@ -421,7 +421,6 @@ namespace std
 */
 
 #include <__config>
-#include <__fwd/functional.h> // This is https://llvm.org/PR56938
 #include <__type_traits/add_const.h>
 #include <__type_traits/add_cv.h>
 #include <__type_traits/add_lvalue_reference.h>
diff --git a/libcxx/test/std/containers/sequences/vector.bool/enabled_hash.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/enabled_hash.pass.cpp
index d89984a4a30bab..66361202f8c003 100644
--- a/libcxx/test/std/containers/sequences/vector.bool/enabled_hash.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector.bool/enabled_hash.pass.cpp
@@ -20,8 +20,8 @@
 #include "min_allocator.h"
 
 TEST_CONSTEXPR_CXX20 bool test() {
-  test_hash_enabled_for_type<std::vector<bool> >();
-  test_hash_enabled_for_type<std::vector<bool, min_allocator<bool>>>();
+  test_hash_enabled<std::vector<bool> >();
+  test_hash_enabled<std::vector<bool, min_allocator<bool>>>();
 
   return true;
 }
diff --git a/libcxx/test/std/diagnostics/syserr/syserr.hash/enabled_hash.pass.cpp b/libcxx/test/std/diagnostics/syserr/syserr.hash/enabled_hash.pass.cpp
index 2aab69883174ae..e3eae8bfa46bbe 100644
--- a/libcxx/test/std/diagnostics/syserr/syserr.hash/enabled_hash.pass.cpp
+++ b/libcxx/test/std/diagnostics/syserr/syserr.hash/enabled_hash.pass.cpp
@@ -22,8 +22,8 @@
 int main(int, char**) {
   test_library_hash_specializations_available();
   {
-    test_hash_enabled_for_type<std::error_code>();
-    test_hash_enabled_for_type<std::error_condition>();
+    test_hash_enabled<std::error_code>();
+    test_hash_enabled<std::error_condition>();
   }
 
   return 0;
diff --git a/libcxx/test/std/experimental/memory/memory.observer.ptr/hash.pass.cpp b/libcxx/test/std/experimental/memory/memory.observer.ptr/hash.pass.cpp
index 7aa5dc8d5b326c..fff5f9bae07337 100644
--- a/libcxx/test/std/experimental/memory/memory.observer.ptr/hash.pass.cpp
+++ b/libcxx/test/std/experimental/memory/memory.observer.ptr/hash.pass.cpp
@@ -33,7 +33,7 @@ void test_hash() {
     assert(h == std::hash<T*>()(&obj));
   }
 
-  test_hash_enabled_for_type<std::experimental::observer_ptr<T>>();
+  test_hash_enabled<std::experimental::observer_ptr<T>>();
 }
 
 struct Bar {};
diff --git a/libcxx/test/std/input.output/filesystems/class.path/path.member/path.hash_enabled.pass.cpp b/libcxx/test/std/input.output/filesystems/class.path/path.member/path.hash_enabled.pass.cpp
index dd28c8f4f9b3de..6cc64e1857d998 100644
--- a/libcxx/test/std/input.output/filesystems/class.path/path.member/path.hash_enabled.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/class.path/path.member/path.hash_enabled.pass.cpp
@@ -20,7 +20,7 @@ namespace fs = std::filesystem;
 
 int main(int, char**) {
   test_library_hash_specializations_available();
-  test_hash_enabled_for_type<fs::path>();
+  test_hash_enabled<fs::path>();
 
   return 0;
 }
diff --git a/libcxx/test/std/strings/basic.string.hash/enabled_hashes.pass.cpp b/libcxx/test/std/strings/basic.string.hash/enabled_hashes.pass.cpp
index 611f95f0d3ef6e..643c6bec637aea 100644
--- a/libcxx/test/std/strings/basic.string.hash/enabled_hashes.pass.cpp
+++ b/libcxx/test/std/strings/basic.string.hash/enabled_hashes.pass.cpp
@@ -53,18 +53,18 @@ struct std::char_traits<MyChar> {
 int main(int, char**) {
   test_library_hash_specializations_available();
   {
-    test_hash_enabled_for_type<std::string>();
+    test_hash_enabled<std::string>();
 #ifndef TEST_HAS_NO_WIDE_CHARACTERS
-    test_hash_enabled_for_type<std::wstring>();
+    test_hash_enabled<std::wstring>();
 #endif
 #ifndef TEST_HAS_NO_CHAR8_T
-    test_hash_enabled_for_type<std::u8string>();
+    test_hash_enabled<std::u8string>();
 #endif
-    test_hash_enabled_for_type<std::u16string>();
-    test_hash_enabled_for_type<std::u32string>();
-    test_hash_enabled_for_type<std::basic_string<char, std::char_traits<char>, test_allocator<char>>>();
-    test_hash_disabled_for_type<std::basic_string<MyChar, std::char_traits<MyChar>, std::allocator<MyChar>>>();
-    test_hash_disabled_for_type<std::basic_string<char, constexpr_char_traits<char>, std::allocator<char>>>();
+    test_hash_enabled<std::u16string>();
+    test_hash_enabled<std::u32string>();
+    test_hash_enabled<std::basic_string<char, std::char_traits<char>, test_allocator<char>>>();
+    test_hash_disabled<std::basic_string<MyChar, std::char_traits<MyChar>, std::allocator<MyChar>>>();
+    test_hash_disabled<std::basic_string<char, constexpr_char_traits<char>, std::allocator<char>>>();
   }
 
   return 0;
diff --git a/libcxx/test/std/strings/string.view/string.view.hash/enabled_hashes.pass.cpp b/libcxx/test/std/strings/string.view/string.view.hash/enabled_hashes.pass.cpp
index b2ffd20108389e..13abb945693682 100644
--- a/libcxx/test/std/strings/string.view/string.view.hash/enabled_hashes.pass.cpp
+++ b/libcxx/test/std/strings/string.view/string.view.hash/enabled_hashes.pass.cpp
@@ -53,17 +53,17 @@ struct std::char_traits<MyChar> {
 int main(int, char**) {
   test_library_hash_specializations_available();
   {
-    test_hash_enabled_for_type<std::string_view>();
+    test_hash_enabled<std::string_view>();
 #ifndef TEST_HAS_NO_WIDE_CHARACTERS
-    test_hash_enabled_for_type<std::wstring_view>();
+    test_hash_enabled<std::wstring_view>();
 #endif
 #ifndef TEST_HAS_NO_CHAR8_T
-    test_hash_enabled_for_type<std::u8string_view>();
+    test_hash_enabled<std::u8string_view>();
 #endif
-    test_hash_enabled_for_type<std::u16string_view>();
-    test_hash_enabled_for_type<std::u32string_view>();
-    test_hash_disabled_for_type<std::basic_string_view<MyChar, std::char_traits<MyChar>>>();
-    test_hash_disabled_for_type<std::basic_string_view<char, constexpr_char_traits<char>>>();
+    test_hash_enabled<std::u16string_view>();
+    test_hash_enabled<std::u32string_view>();
+    test_hash_disabled<std::basic_string_view<MyChar, std::char_traits<MyChar>>>();
+    test_hash_disabled<std::basic_string_view<char, constexpr_char_traits<char>>>();
   }
 
   return 0;
diff --git a/libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/enabled_hashes.pass.cpp b/libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/enabled_hashes.pass.cpp
index 62c8c7476ab7ec..98caff904916a7 100644
--- a/libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/enabled_hashes.pass.cpp
+++ b/libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/enabled_hashes.pass.cpp
@@ -24,7 +24,7 @@
 int main(int, char**) {
   test_library_hash_specializations_available();
   {
-    test_hash_enabled_for_type<std::thread::id>();
+    test_hash_enabled<std::thread::id>();
   }
 
   return 0;
diff --git a/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.hash/hash_shared_ptr.pass.cpp b/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.hash/hash_shared_ptr.pass.cpp
index 0c3915bf480419..c6d54a82fecaa5 100644
--- a/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.hash/hash_shared_ptr.pass.cpp
+++ b/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.hash/hash_shared_ptr.pass.cpp
@@ -40,8 +40,8 @@ int main(int, char**)
   }
 #if TEST_STD_VER >= 11
   {
-    test_hash_enabled_for_type<std::shared_ptr<int>>();
-    test_hash_enabled_for_type<std::shared_ptr<A>>();
+    test_hash_enabled<std::shared_ptr<int>>();
+    test_hash_enabled<std::shared_ptr<A>>();
   }
 #endif
 
diff --git a/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.hash/hash_unique_ptr.pass.cpp b/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.hash/hash_unique_ptr.pass.cpp
index 707038e53ed102..32fc949354c69a 100644
--- a/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.hash/hash_unique_ptr.pass.cpp
+++ b/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.hash/hash_unique_ptr.pass.cpp
@@ -35,16 +35,16 @@ void test_enabled_with_deleter() {
   using RawDel = typename std::decay<Del>::type;
   RawDel d(1);
   UPtr p(nullptr, std::forward<Del>(d));
-  test_hash_enabled_for_type<UPtr>(p);
-  test_hash_enabled_for_type<pointer>();
+  test_hash_enabled<UPtr>(p);
+  test_hash_enabled<pointer>();
 }
 
 template <class ValueT, class Del>
 void test_disabled_with_deleter() {
   using UPtr = std::unique_ptr<ValueT, Del>;
   using pointer = typename UPtr::pointer;
-  test_hash_disabled_for_type<UPtr>();
-  test_hash_disabled_for_type<pointer>();
+  test_hash_disabled<UPtr>();
+  test_hash_disabled<pointer>();
 }
 
 template <class T>
diff --git a/libcxx/test/std/utilities/optional/optional.hash/hash.pass.cpp b/libcxx/test/std/utilities/optional/optional.hash/hash.pass.cpp
index ae14b571f7388b..54cf40740835d4 100644
--- a/libcxx/test/std/utilities/optional/optional.hash/hash.pass.cpp
+++ b/libcxx/test/std/utilities/optional/optional.hash/hash.pass.cpp
@@ -63,16 +63,16 @@ int main(int, char**)
         assert(std::hash<optional<T>>{}(opt) == std::hash<T>{}(*opt));
     }
     {
-      test_hash_enabled_for_type<std::optional<int> >();
-      test_hash_enabled_for_type<std::optional<int*> >();
-      test_hash_enabled_for_type<std::optional<const int> >();
-      test_hash_enabled_for_type<std::optional<int* const> >();
+      test_hash_enabled<std::optional<int> >();
+      test_hash_enabled<std::optional<int*> >();
+      test_hash_enabled<std::optional<const int> >();
+      test_hash_enabled<std::optional<int* const> >();
 
-      test_hash_disabled_for_type<std::optional<A>>();
-      test_hash_disabled_for_type<std::optional<const A>>();
+      test_hash_disabled<std::optional<A>>();
+      test_hash_disabled<std::optional<const A>>();
 
-      test_hash_enabled_for_type<std::optional<B>>();
-      test_hash_enabled_for_type<std::optional<const B>>();
+      test_hash_enabled<std::optional<B>>();
+      test_hash_enabled<std::optional<const B>>();
     }
 
   return 0;
diff --git a/libcxx/test/std/utilities/template.bitset/bitset.hash/enabled_hash.pass.cpp b/libcxx/test/std/utilities/template.bitset/bitset.hash/enabled_hash.pass.cpp
index 0e34a5f97897b2..c2dc2ca8935636 100644
--- a/libcxx/test/std/utilities/template.bitset/bitset.hash/enabled_hash.pass.cpp
+++ b/libcxx/test/std/utilities/template.bitset/bitset.hash/enabled_hash.pass.cpp
@@ -22,10 +22,10 @@
 int main(int, char**) {
   test_library_hash_specializations_available();
   {
-    test_hash_enabled_for_type<std::bitset<0> >();
-    test_hash_enabled_for_type<std::bitset<1> >();
-    test_hash_enabled_for_type<std::bitset<1024> >();
-    test_hash_enabled_for_type<std::bitset<100000> >();
+    test_hash_enabled<std::bitset<0> >();
+    test_hash_enabled<std::bitset<1> >();
+    test_hash_enabled<std::bitset<1024> >();
+    test_hash_enabled<std::bitset<100000> >();
   }
 
   return 0;
diff --git a/libcxx/test/std/utilities/type.index/type.index.synopsis/hash_type_index.pass.cpp b/libcxx/test/std/utilities/type.index/type.index.synopsis/hash_type_index.pass.cpp
index a36175875b6953..9c0de17837e6f5 100644
--- a/libcxx/test/std/utilities/type.index/type.index.synopsis/hash_type_index.pass.cpp
+++ b/libcxx/test/std/utilities/type.index/type.index.synopsis/hash_type_index.pass.cpp
@@ -34,7 +34,7 @@ int main(int, char**)
   }
 #if TEST_STD_VER >= 11
   {
-    test_hash_enabled_for_type<std::type_index>(std::type_index(typeid(int)));
+    test_hash_enabled<std::type_index>(std::type_index(typeid(int)));
   }
 #endif
 
diff --git a/libcxx/test/std/utilities/variant/variant.hash/hash.pass.cpp b/libcxx/test/std/utilities/variant/variant.hash/hash.pass.cpp
index ffd5f8266ec2bf..656b1d83c58c6c 100644
--- a/libcxx/test/std/utilities/variant/variant.hash/hash.pass.cpp
+++ b/libcxx/test/std/utilities/variant/variant.hash/hash.pass.cpp
@@ -103,7 +103,7 @@ void test_hash_monostate() {
     static_assert(std::is_copy_constructible<H>::value, "");
   }
   {
-    test_hash_enabled_for_type<std::monostate>();
+    test_hash_enabled<std::monostate>();
   }
 }
 
@@ -131,16 +131,16 @@ struct std::hash<B> {
 
 void test_hash_variant_enabled() {
   {
-    test_hash_enabled_for_type<std::variant<int> >();
-    test_hash_enabled_for_type<std::variant<int*, long, double, const int> >();
+    test_hash_enabled<std::variant<int> >();
+    test_hash_enabled<std::variant<int*, long, double, const int> >();
   }
   {
-    test_hash_disabled_for_type<std::variant<int, A>>();
-    test_hash_disabled_for_type<std::variant<const A, void*>>();
+    test_hash_disabled<std::variant<int, A>>();
+    test_hash_disabled<std::variant<const A, void*>>();
   }
   {
-    test_hash_enabled_for_type<std::variant<int, B>>();
-    test_hash_enabled_for_type<std::variant<const B, int>>();
+    test_hash_enabled<std::variant<int, B>>();
+    test_hash_enabled<std::variant<const B, int>>();
   }
 }
 
diff --git a/libcxx/test/support/poisoned_hash_helper.h b/libcxx/test/support/poisoned_hash_helper.h
index a073350c1470e7..3abac06938e05a 100644
--- a/libcxx/test/support/poisoned_hash_helper.h
+++ b/libcxx/test/support/poisoned_hash_helper.h
@@ -10,131 +10,47 @@
 #ifndef SUPPORT_POISONED_HASH_HELPER_H
 #define SUPPORT_POISONED_HASH_HELPER_H
 
+#include <functional>
 #include <cassert>
 #include <cstddef>
 #include <type_traits>
 #include <utility>
 
 #include "test_macros.h"
-#include "test_workarounds.h"
+#include "type_algorithms.h"
 
-#if TEST_STD_VER < 11
-#error this header may only be used in C++11 or newer
-#endif
-
-template <class ...Args> struct TypeList;
-
-// Test that the specified Hash meets the requirements of an enabled hash
-template <class Hash, class Key, class InputKey = Key>
-TEST_CONSTEXPR_CXX20 void test_hash_enabled(InputKey const& key = InputKey{});
-
-template <class T, class InputKey = T>
-TEST_CONSTEXPR_CXX20 void test_hash_enabled_for_type(InputKey const& key = InputKey{}) {
-  return test_hash_enabled<std::hash<T>, T, InputKey>(key);
+template <class Hash, class Key, class Res = decltype(std::declval<Hash&>()(std::declval<Key>()))>
+constexpr bool can_hash_impl(int) {
+  return std::is_same<Res, std::size_t>::value;
 }
-
-// Test that the specified Hash meets the requirements of a disabled hash.
-template <class Hash, class Key>
-void test_hash_disabled();
-
-template <class T>
-void test_hash_disabled_for_type() {
-  return test_hash_disabled<std::hash<T>, T>();
+template <class, class>
+constexpr bool can_hash_impl(long) {
+  return false;
 }
-
-namespace PoisonedHashDetail {
-  enum Enum {};
-  enum EnumClass : bool {};
-  struct Class {};
+template <class Hash, class Key>
+constexpr bool can_hash() {
+  return can_hash_impl<Hash, Key>(0);
 }
 
-// Each header that declares the template hash provides enabled
-// specializations of hash for nullptr t and all cv-unqualified
-// arithmetic, enumeration, and pointer types.
-using LibraryHashTypes = TypeList<
-#if TEST_STD_VER > 14
-      decltype(nullptr),
-#endif
-      bool,
-      char,
-      signed char,
-      unsigned char,
-#ifndef TEST_HAS_NO_WIDE_CHARACTERS
-      wchar_t,
-#endif
-      char16_t,
-      char32_t,
-      short,
-      unsigned short,
-      int,
-      unsigned int,
-      long,
-      unsigned long,
-      long long,
-      unsigned long long,
-#ifndef TEST_HAS_NO_INT128
-      __int128_t,
-      __uint128_t,
-#endif
-      float,
-      double,
-      long double,
-      PoisonedHashDetail::Enum,
-      PoisonedHashDetail::EnumClass,
-      void*,
-      void const*,
-      PoisonedHashDetail::Class*
-    >;
-
-
-// Test that each of the library hash specializations for  arithmetic types,
-// enum types, and pointer types are available and enabled.
-template <class Types = LibraryHashTypes>
-void test_library_hash_specializations_available(Types = Types{});
-
-
-namespace PoisonedHashDetail {
-
-template <class T, class = typename T::foo_bar_baz>
-constexpr bool instantiate(int) { return true; }
-template <class> constexpr bool instantiate(long) { return true; }
-template <class T> constexpr bool instantiate() { return instantiate<T>(0); }
-
 template <class To>
 struct ConvertibleToSimple {
-  operator To() const {
-    return To{};
-  }
+  operator To() const { return To{}; }
 };
 
 template <class To>
 struct ConvertibleTo {
   To to{};
   operator To&() & { return to; }
-  operator To const&() const & { return to; }
+  operator To const&() const& { return to; }
   operator To&&() && { return std::move(to); }
-  operator To const&&() const && { return std::move(to); }
+  operator To const&&() const&& { return std::move(to); }
 };
 
-template <class Hasher, class Key, class Res = decltype(std::declval<Hasher&>()(std::declval<Key>()))>
-constexpr bool can_hash(int) {
-  return std::is_same<Res, std::size_t>::value;
-}
-template <class, class>
-constexpr bool can_hash(long) {
-  return false;
-}
-template <class Hasher, class Key>
-constexpr bool can_hash() {
-  return can_hash<Hasher, Key>(0);
-}
-} // namespace PoisonedHashDetail
-
-template <class Hash, class Key, class InputKey>
-TEST_CONSTEXPR_CXX20 void test_hash_enabled(InputKey const& key) {
-  using namespace PoisonedHashDetail;
-
+// Test that the specified Hash meets the requirements of an enabled hash
+template <class Key, class Hash = std::hash<Key>>
+TEST_CONSTEXPR_CXX20 void test_hash_enabled(Key const& key = Key{}) {
   static_assert(std::is_destructible<Hash>::value, "");
+
   // Enabled hash requirements
   static_assert(std::is_default_constructible<Hash>::value, "");
   static_assert(std::is_copy_constructible<Hash>::value, "");
@@ -167,13 +83,11 @@ TEST_CONSTEXPR_CXX20 void test_hash_enabled(InputKey const& key) {
 
   const Hash h{};
   assert(h(key) == h(key));
-
 }
 
-template <class Hash, class Key>
+// Test that the specified Hash meets the requirements of a disabled hash.
+template <class Key, class Hash = std::hash<Key>>
 void test_hash_disabled() {
-  using namespace PoisonedHashDetail;
-
   // Disabled hash requirements
   static_assert(!std::is_default_constructible<Hash>::value, "");
   static_assert(!std::is_copy_constructible<Hash>::value, "");
@@ -181,11 +95,8 @@ void test_hash_disabled() {
   static_assert(!std::is_copy_assignable<Hash>::value, "");
   static_assert(!std::is_move_assignable<Hash>::value, "");
 
-  static_assert(!std::is_function<
-      typename std::remove_pointer<
-          typename std::remove_reference<Hash>::type
-      >::type
-    >::value, "");
+  static_assert(
+      !std::is_function<typename std::remove_pointer<typename std::remove_reference<Hash>::type>::type>::value, "");
 
   // Hashable requirements
   static_assert(!can_hash<Hash, Key&>(), "");
@@ -205,41 +116,59 @@ void test_hash_disabled() {
   static_assert(!can_hash<Hash, ConvertibleTo<Key> const&&>(), "");
 }
 
+enum Enum {};
+enum EnumClass : bool {};
+struct Class {};
 
-template <class First, class ...Rest>
-struct TypeList<First, Rest...> {
-  template <template <class> class Trait, bool Expect = true>
-  static constexpr bool assertTrait() {
-    static_assert(Trait<First>::value == Expect, "");
-    return TypeList<Rest...>::template assertTrait<Trait, Expect>();
-  }
-
-  template <class Trait>
-  static void applyTrait() {
-    Trait::template apply<First>();
-    TypeList<Rest...>::template applyTrait<Trait>();
-  }
-};
-
-template <>
-struct TypeList<> {
-  template <template <class> class Trait, bool Expect = true>
-  static constexpr bool assertTrait() {
-    return true;
+// Each header that declares the std::hash template provides enabled
+// specializations of std::hash for std::nullptr_t and all cv-unqualified
+// arithmetic, enumeration, and pointer types.
+using LibraryHashTypes = types::type_list<
+#if TEST_STD_VER > 14
+    decltype(nullptr),
+#endif
+    bool,
+    char,
+    signed char,
+    unsigned char,
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+    wchar_t,
+#endif
+    char16_t,
+    char32_t,
+    short,
+    unsigned short,
+    int,
+    unsigned int,
+    long,
+    unsigned long,
+    long long,
+    unsigned long long,
+#ifndef TEST_HAS_NO_INT128
+    __int128_t,
+    __uint128_t,
+#endif
+    float,
+    double,
+    long double,
+    Enum,
+    EnumClass,
+    void*,
+    void const*,
+    Class*>;
+
+struct TestHashEnabled {
+  template <class T>
+  void operator()() const {
+    test_hash_enabled<T>();
   }
-  template <class Trait>
-  static void applyTrait() {}
 };
 
-
-struct TestLibraryTrait {
-    ...
[truncated]

Copy link

github-actions bot commented Sep 11, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 118f120eaab8d763b28c71f0d2e2c1e0c752832b 26985a90339b123885c023ff29231ec8d2f7cc26 --extensions ,h,cpp -- libcxx/include/type_traits libcxx/test/std/containers/sequences/vector.bool/enabled_hash.pass.cpp libcxx/test/std/diagnostics/syserr/syserr.hash/enabled_hash.pass.cpp libcxx/test/std/experimental/memory/memory.observer.ptr/hash.pass.cpp libcxx/test/std/input.output/filesystems/class.path/path.member/path.hash_enabled.pass.cpp libcxx/test/std/strings/basic.string.hash/enabled_hashes.pass.cpp libcxx/test/std/strings/string.view/string.view.hash/enabled_hashes.pass.cpp libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/enabled_hashes.pass.cpp libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.hash/hash_shared_ptr.pass.cpp libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.hash/hash_unique_ptr.pass.cpp libcxx/test/std/utilities/optional/optional.hash/hash.pass.cpp libcxx/test/std/utilities/template.bitset/bitset.hash/enabled_hash.pass.cpp libcxx/test/std/utilities/type.index/type.index.synopsis/hash_type_index.pass.cpp libcxx/test/std/utilities/variant/variant.hash/hash.pass.cpp libcxx/test/support/poisoned_hash_helper.h
View the diff from clang-format here.
diff --git a/libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/enabled_hashes.pass.cpp b/libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/enabled_hashes.pass.cpp
index 98caff9049..a4b1155611 100644
--- a/libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/enabled_hashes.pass.cpp
+++ b/libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/enabled_hashes.pass.cpp
@@ -23,9 +23,7 @@
 
 int main(int, char**) {
   test_library_hash_specializations_available();
-  {
-    test_hash_enabled<std::thread::id>();
-  }
+  { test_hash_enabled<std::thread::id>(); }
 
   return 0;
 }
diff --git a/libcxx/test/std/utilities/type.index/type.index.synopsis/hash_type_index.pass.cpp b/libcxx/test/std/utilities/type.index/type.index.synopsis/hash_type_index.pass.cpp
index 9c0de17837..8a6a48f0e2 100644
--- a/libcxx/test/std/utilities/type.index/type.index.synopsis/hash_type_index.pass.cpp
+++ b/libcxx/test/std/utilities/type.index/type.index.synopsis/hash_type_index.pass.cpp
@@ -33,9 +33,7 @@ int main(int, char**)
 #endif
   }
 #if TEST_STD_VER >= 11
-  {
-    test_hash_enabled<std::type_index>(std::type_index(typeid(int)));
-  }
+  { test_hash_enabled<std::type_index>(std::type_index(typeid(int))); }
 #endif
 
   return 0;
diff --git a/libcxx/test/std/utilities/variant/variant.hash/hash.pass.cpp b/libcxx/test/std/utilities/variant/variant.hash/hash.pass.cpp
index 656b1d83c5..0fc2f37807 100644
--- a/libcxx/test/std/utilities/variant/variant.hash/hash.pass.cpp
+++ b/libcxx/test/std/utilities/variant/variant.hash/hash.pass.cpp
@@ -102,9 +102,7 @@ void test_hash_monostate() {
     ASSERT_NOEXCEPT(h(m1));
     static_assert(std::is_copy_constructible<H>::value, "");
   }
-  {
-    test_hash_enabled<std::monostate>();
-  }
+  { test_hash_enabled<std::monostate>(); }
 }
 
 void test_hash_variant_duplicate_elements() {

libcxx/test/support/poisoned_hash_helper.h Outdated Show resolved Hide resolved
@ldionne
Copy link
Member Author

ldionne commented Sep 12, 2024

Merging since this passed previously and the CI for all standard modes and the modular build passed.

@ldionne ldionne merged commit 3332552 into llvm:main Sep 12, 2024
56 of 62 checks passed
@ldionne ldionne deleted the review/modularization-fix-poisoned-hash-helper branch September 12, 2024 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

poisoned_hash_helper.h relies on forward-declaring std::hash
3 participants