diff mbox

[v3] Make poisoned hashes SFINAE away the call operator of the hash.

Message ID CAFk2RUZ1H_Q4WJ5WccGjSmxqmWX6mkLy5veRDHHHQKAPH3ABLg@mail.gmail.com
State New
Headers show

Commit Message

Ville Voutilainen Jan. 20, 2017, 3:50 p.m. UTC
On 20 January 2017 at 17:27, Ville Voutilainen
<ville.voutilainen@gmail.com> wrote:
> On 20 January 2017 at 17:21, Jonathan Wakely <jwakely@redhat.com> wrote:
>> This part for std::unique_ptr is the only part that isn't C++17-only,
>> so this is the part I'm most nervous about. Given that we're in stage
>> 4 and this isn't even fixing something in Bugzilla (even if it is a
>> real bug), would it be possible to only fix optional and variant for
>> now? We could revisit the unique_ptr part once we know this approach
>> doesn't have some other problem that we haven't guessed yet.
>
> Sure. I would guesstimate that custom pointer types that aren't
> hashable should be rare,
> so hitting that problem is unlikely.
>
> I'll remove the unique_ptr bits, will give the deduced-bool a spin and
> fix the whitespace, and will then
> send a new patch for review.

Here:

2017-01-20  Ville Voutilainen  <ville.voutilainen@gmail.com>

2017-01-20  Ville Voutilainen  <ville.voutilainen@gmail.com>

    Make poisoned hashes SFINAE away the call operator
    of the hash.
    * include/bits/functional_hash.h
    (__poison_hash::__enable_hash_call): New.
    * include/std/optional (__optional_hash_call_base): New.
    (hash<optional<_Tp>>): Derive from the new base,
    move the hash function into that base.
    * include/std/variant (__variant_hash_call_base_impl): New.
    (__variant_hash_call_base): Likewise.
    (hash<variant<_Types...>>): Derive from the new base,
    move the hash function into that base.
    * testsuite/20_util/optional/hash.cc: Add tests for is_callable.
    * testsuite/20_util/variant/hash.cc: Likewise.

Comments

Jonathan Wakely Jan. 20, 2017, 4:39 p.m. UTC | #1
On 20/01/17 17:50 +0200, Ville Voutilainen wrote:
>On 20 January 2017 at 17:27, Ville Voutilainen
><ville.voutilainen@gmail.com> wrote:
>> On 20 January 2017 at 17:21, Jonathan Wakely <jwakely@redhat.com> wrote:
>>> This part for std::unique_ptr is the only part that isn't C++17-only,
>>> so this is the part I'm most nervous about. Given that we're in stage
>>> 4 and this isn't even fixing something in Bugzilla (even if it is a
>>> real bug), would it be possible to only fix optional and variant for
>>> now? We could revisit the unique_ptr part once we know this approach
>>> doesn't have some other problem that we haven't guessed yet.
>>
>> Sure. I would guesstimate that custom pointer types that aren't
>> hashable should be rare,
>> so hitting that problem is unlikely.
>>
>> I'll remove the unique_ptr bits, will give the deduced-bool a spin and
>> fix the whitespace, and will then
>> send a new patch for review.
>
>Here:

OK for trunk - thanks.
diff mbox

Patch

diff --git a/libstdc++-v3/include/bits/functional_hash.h b/libstdc++-v3/include/bits/functional_hash.h
index 14f7fae..38be172 100644
--- a/libstdc++-v3/include/bits/functional_hash.h
+++ b/libstdc++-v3/include/bits/functional_hash.h
@@ -60,6 +60,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template<typename _Tp, typename = void>
     struct __poison_hash
     {
+      static constexpr bool __enable_hash_call = false;
     private:
       // Private rather than deleted to be non-trivially-copyable.
       __poison_hash(__poison_hash&&);
@@ -69,6 +70,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template<typename _Tp>
     struct __poison_hash<_Tp, __void_t<decltype(hash<_Tp>()(declval<_Tp>()))>>
     {
+      static constexpr bool __enable_hash_call = true;
     };
 
   // Helper struct for SFINAE-poisoning non-enum types.
diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
index 85ec91d..887bf9e 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -951,12 +951,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     { return optional<_Tp> { in_place, __il, std::forward<_Args>(__args)... }; }
 
   // Hash.
-  template<typename _Tp>
-    struct hash<optional<_Tp>> : private __poison_hash<remove_const_t<_Tp>>
-    {
-      using result_type = size_t;
-      using argument_type = optional<_Tp>;
 
+  template<typename _Tp, bool
+           = __poison_hash<remove_const_t<_Tp>>::__enable_hash_call>
+    struct __optional_hash_call_base
+    {
       size_t
       operator()(const optional<_Tp>& __t) const
       noexcept(noexcept(hash<_Tp> {}(*__t)))
@@ -968,6 +967,18 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }
     };
 
+  template<typename _Tp>
+    struct __optional_hash_call_base<_Tp, false> {};
+
+  template<typename _Tp>
+    struct hash<optional<_Tp>>
+    : private __poison_hash<remove_const_t<_Tp>>,
+      public __optional_hash_call_base<_Tp>
+    {
+      using result_type = size_t;
+      using argument_type = optional<_Tp>;
+    };
+
   /// @}
 
 _GLIBCXX_END_NAMESPACE_VERSION
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 6404fce..c5138e5 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -1273,14 +1273,9 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 			   std::forward<_Variants>(__variants)...);
     }
 
-  template<typename... _Types>
-    struct hash<variant<_Types...>>
-    : private __detail::__variant::_Variant_hash_base<
-	variant<_Types...>, std::index_sequence_for<_Types...>>
+  template<bool, typename... _Types>
+    struct __variant_hash_call_base_impl
     {
-      using result_type = size_t;
-      using argument_type = variant<_Types...>;
-
       size_t
       operator()(const variant<_Types...>& __t) const
       noexcept((is_nothrow_callable_v<hash<decay_t<_Types>>(_Types)> && ...))
@@ -1297,6 +1292,24 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }
     };
 
+  template<typename... _Types>
+    struct __variant_hash_call_base_impl<false, _Types...> {};
+
+  template<typename... _Types>
+    using __variant_hash_call_base =
+    __variant_hash_call_base_impl<(__poison_hash<remove_const_t<_Types>>::
+				   __enable_hash_call &&...), _Types...>;
+
+  template<typename... _Types>
+    struct hash<variant<_Types...>>
+    : private __detail::__variant::_Variant_hash_base<
+        variant<_Types...>, std::index_sequence_for<_Types...>>,
+      public __variant_hash_call_base<_Types...>
+    {
+      using result_type = size_t;
+      using argument_type = variant<_Types...>;
+    };
+
   template<>
     struct hash<monostate>
     {
diff --git a/libstdc++-v3/testsuite/20_util/optional/hash.cc b/libstdc++-v3/testsuite/20_util/optional/hash.cc
index ceb862b..297ea2e 100644
--- a/libstdc++-v3/testsuite/20_util/optional/hash.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/hash.cc
@@ -29,6 +29,12 @@  template<class T>
 auto f(...) -> decltype(std::false_type());
 
 static_assert(!decltype(f<S>(0))::value, "");
+static_assert(!std::is_callable<
+	      std::hash<std::optional<S>>&
+	      (std::optional<S> const&)>::value, "");
+static_assert(std::is_callable<
+	      std::hash<std::optional<int>>&
+	      (std::optional<int> const&)>::value, "");
 
 int main()
 {
diff --git a/libstdc++-v3/testsuite/20_util/variant/hash.cc b/libstdc++-v3/testsuite/20_util/variant/hash.cc
index a9ebf33..0a267ab 100644
--- a/libstdc++-v3/testsuite/20_util/variant/hash.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/hash.cc
@@ -33,6 +33,17 @@  static_assert(!decltype(f<std::variant<S>>(0))::value, "");
 static_assert(!decltype(f<std::variant<S, S>>(0))::value, "");
 static_assert(decltype(f<std::variant<int>>(0))::value, "");
 static_assert(decltype(f<std::variant<int, int>>(0))::value, "");
+static_assert(!std::is_callable<
+	      std::hash<std::variant<S>>&(std::variant<S> const&)>::value, "");
+static_assert(!std::is_callable<
+	      std::hash<std::variant<S, int>>&
+	      (std::variant<S, int> const&)>::value, "");
+static_assert(std::is_callable<
+	      std::hash<std::variant<int>>&
+	      (std::variant<int> const&)>::value, "");
+static_assert(std::is_callable<
+	      std::hash<std::variant<int, int>>&
+	      (std::variant<int, int> const&)>::value, "");
 
 int main()
 {