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() {