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 <[email protected]>
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()
{
