On 20 January 2017 at 17:27, Ville Voutilainen <ville.voutilai...@gmail.com> wrote: > On 20 January 2017 at 17:21, Jonathan Wakely <jwak...@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.voutilai...@gmail.com> 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/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.
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() {