Tested on Linux-x64. The approach is a bit ugly, the hashes derive
from __poison_hash and also use its nested __enable_hash_call as
an argument for another utility base. I prototyped various approaches
to try to decrease that duplication, but didn't like any of them (more
utility bases, aliases, even passing a lambda that does the concrete
hash to a more generic hash in a base), and they certainly didn't make
the code any shorter.

The reason this patch was done is that K-ballo pointed out that
our hashes have a non-SFINAED call operator that uses a constructing-expression
in its noexcept-spec - so asking is_callable from a poisoned hash leads
to a hard error, including in cases where the user would correctly
check both callability and constructibility, but in that order. I steered
away from turning the call operators into constrained templates, and
chose a conditional base approach instead, because that keeps functions
as functions instead of turning them into templates; we do not want
to open the possibility that a caller starts providing template arguments
explicitly.

This patch covers unique_ptr, std::optional and std::variant. We might
want to entertain doing this for std::experimental::optional as well.

2017-01-20  Ville Voutilainen  <ville.voutilai...@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/bits/unique_ptr.h (__unique_ptr_hash_call_base): New.
    (hash<unique_ptr<_Tp, _Dp>>): Derive from the new base,
    move the hash function into that base, use the added enabling
    flag to SFINAE.
    * include/std/optional (__optional_hash_call_base): New.
    (hash<optional<_Tp>>): Derive from the new base,
    move the hash function into that base, use the added enabling
    flag to SFINAE.
    * include/std/variant(__variant_hash_call_base): New.
    (hash<variant<_Types...>>): Derive from the new base,
    move the hash function into that base, use the added enabling
    flag to SFINAE.
    * testsuite/20_util/optional/hash.cc: Add tests for is_callable.
    * testsuite/20_util/unique_ptr/hash/1.cc: Likewise.
    * testsuite/20_util/variant/hash.cc: Likewise.
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/bits/unique_ptr.h 
b/libstdc++-v3/include/bits/unique_ptr.h
index a31cd67..f4763a0 100644
--- a/libstdc++-v3/include/bits/unique_ptr.h
+++ b/libstdc++-v3/include/bits/unique_ptr.h
@@ -789,10 +789,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     { return !(nullptr < __x); }
 
   /// std::hash specialization for unique_ptr.
-  template<typename _Tp, typename _Dp>
-    struct hash<unique_ptr<_Tp, _Dp>>
-    : public __hash_base<size_t, unique_ptr<_Tp, _Dp>>,
-    private __poison_hash<typename unique_ptr<_Tp, _Dp>::pointer>
+
+  template<typename _Tp, typename _Dp, bool>
+    struct __unique_ptr_hash_call_base
     {
       size_t
       operator()(const unique_ptr<_Tp, _Dp>& __u) const noexcept
@@ -802,6 +801,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }
     };
 
+  template<typename _Tp, typename _Dp>
+    struct __unique_ptr_hash_call_base<_Tp, _Dp, false> {};
+
+  template<typename _Tp, typename _Dp>
+    struct hash<unique_ptr<_Tp, _Dp>>
+    : public __hash_base<size_t, unique_ptr<_Tp, _Dp>>,
+    private __poison_hash<typename unique_ptr<_Tp, _Dp>::pointer>,
+    public __unique_ptr_hash_call_base<_Tp, _Dp,
+      __poison_hash<typename unique_ptr<_Tp, _Dp>::pointer>::
+        __enable_hash_call>
+    {
+    };
+
 #if __cplusplus > 201103L
 
 #define __cpp_lib_make_unique 201304
diff --git a/libstdc++-v3/include/std/optional 
b/libstdc++-v3/include/std/optional
index 85ec91d..85c95b1 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -951,21 +951,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     { return optional<_Tp> { in_place, __il, std::forward<_Args>(__args)... }; 
}
 
   // Hash.
+
+  template<typename _Tp, bool>
+    struct __optional_hash_call_base
+  {
+    size_t
+    operator()(const optional<_Tp>& __t) const
+      noexcept(noexcept(hash<_Tp> {}(*__t)))
+    {
+      // We pick an arbitrary hash for disengaged optionals which hopefully
+      // usual values of _Tp won't typically hash to.
+      constexpr size_t __magic_disengaged_hash = static_cast<size_t>(-3333);
+      return __t ? hash<_Tp> {}(*__t) : __magic_disengaged_hash;
+    }
+  };
+
   template<typename _Tp>
-    struct hash<optional<_Tp>> : private __poison_hash<remove_const_t<_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,
+                                 __poison_hash<remove_const_t<_Tp>>::
+                                   __enable_hash_call>
     {
       using result_type = size_t;
       using argument_type = optional<_Tp>;
-
-      size_t
-      operator()(const optional<_Tp>& __t) const
-      noexcept(noexcept(hash<_Tp> {}(*__t)))
-      {
-        // We pick an arbitrary hash for disengaged optionals which hopefully
-        // usual values of _Tp won't typically hash to.
-        constexpr size_t __magic_disengaged_hash = static_cast<size_t>(-3333);
-        return __t ? hash<_Tp> {}(*__t) : __magic_disengaged_hash;
-      }
     };
 
   /// @}
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 6404fce..0ff8439 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
     {
-      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,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }
     };
 
+  template<typename... _Types>
+    struct __variant_hash_call_base<false, _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<(
+      __poison_hash<remove_const_t<_Types>>::
+        __enable_hash_call &&...), _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/unique_ptr/hash/1.cc 
b/libstdc++-v3/testsuite/20_util/unique_ptr/hash/1.cc
index dfe8a69..dcd3f9e 100644
--- a/libstdc++-v3/testsuite/20_util/unique_ptr/hash/1.cc
+++ b/libstdc++-v3/testsuite/20_util/unique_ptr/hash/1.cc
@@ -44,6 +44,13 @@ template<class T>
 auto f(...) -> decltype(std::false_type());
 
 static_assert(!decltype(f<Pointer<int>>(0))::value, "");
+static_assert(!std::__is_callable<
+             std::hash<std::unique_ptr<Pointer<int>, PointerDeleter<int>>>&
+             (std::unique_ptr<Pointer<int>, PointerDeleter<int>> const&)>
+             ::value, "");
+static_assert(std::__is_callable<
+             std::hash<std::unique_ptr<int>>&
+             (std::unique_ptr<int> const&)>::value, "");
 
 void test01()
 {
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()
 {

Reply via email to