EricWF created this revision.
This is my attempt to improve the diagnostic messages emitted from the
unordered containers when the supplied hash doesn't meet the `Hash`
requirements.
Unfortunately producing reasonable diagnostics is tricky and it requires a
bunch of meta-programming to get right.
@thakis Does this look more reasonable to you?
https://reviews.llvm.org/D30597
Files:
include/__hash_table
include/utility
test/libcxx/containers/unord/unord.set/missing_hash_specialization.fail.cpp
Index: test/libcxx/containers/unord/unord.set/missing_hash_specialization.fail.cpp
===================================================================
--- test/libcxx/containers/unord/unord.set/missing_hash_specialization.fail.cpp
+++ test/libcxx/containers/unord/unord.set/missing_hash_specialization.fail.cpp
@@ -10,18 +10,16 @@
// REQUIRES: diagnose-if-support
// UNSUPPORTED: c++98, c++03
-// Libc++ only provides a defined primary template for std::hash in C++14 and
-// newer.
-// UNSUPPORTED: c++11
-
// <unordered_set>
// Test that we generate a reasonable diagnostic when the specified hash is
// not enabled.
#include <unordered_set>
#include <utility>
+#include "test_macros.h"
+
using VT = std::pair<int, int>;
struct BadHashNoCopy {
@@ -47,21 +45,22 @@
{
using Set = std::unordered_set<VT>;
- Set s; // expected-error@__hash_table:* {{the specified hash does not meet the Hash requirements}}
-
-
- // FIXME: It would be great to suppress the below diagnostic all together.
- // but for now it's sufficient that it appears last. However there is
- // currently no way to test the order diagnostics are issued.
- // expected-error@memory:* {{call to implicitly-deleted default constructor of 'std::__1::hash<std::__1::pair<int, int> >'}}
+ Set s; // expected-error@utility:* {{no matching specialization for std::hash<Key>}}
+#if TEST_STD_VER == 11
+ // expected-error@type_traits:* {{implicit instantiation of undefined template}}
+ // expected-error@memory:* {{no member named 'value'}}
+ // expected-error@memory:* {{multiple overloads of '__compressed_pair'}}
+#else
+ // expected-error@memory:* {{call to implicitly-deleted default constructor}}
+#endif
}
{
using Set = std::unordered_set<int, BadHashNoCopy>;
- Set s; // expected-error@__hash_table:* {{the specified hash does not meet the Hash requirements}}
+ Set s; // expected-error@utility:* {{the specified hash is required to be copy constructible}}
}
{
using Set = std::unordered_set<int, BadHashNoCall>;
- Set s; // expected-error@__hash_table:* {{the specified hash does not meet the Hash requirements}}
+ Set s; // expected-error@utility:* {{the specified hash is required to be callable type}}
}
{
using Set = std::unordered_set<int, GoodHashNoDefault>;
Index: include/utility
===================================================================
--- include/utility
+++ include/utility
@@ -1535,6 +1535,7 @@
};
template <class _Tp>
struct _LIBCPP_TEMPLATE_VIS __enum_hash<_Tp, false> {
+ using __is_invalid_std_hash = true_type;
__enum_hash() = delete;
__enum_hash(__enum_hash const&) = delete;
__enum_hash& operator=(__enum_hash const&) = delete;
@@ -1560,16 +1561,90 @@
#endif
#ifndef _LIBCPP_CXX03_LANG
+#if _LIBCPP_STD_VER > 11
+template <class _Hash, class = void>
+struct __is_invalid_std_hash : false_type {};
+
+template <class _Arg>
+struct __is_invalid_std_hash<
+ _Arg,
+ typename __void_t<typename _Arg::__is_invalid_std_hash>::type
+> : true_type {};
+#else
+template <class _Hash>
+struct __is_hash_specialization : false_type {};
+
+template <class _Arg>
+struct __is_hash_specialization<hash<_Arg>> : true_type {};
+
+template <class _Hash, class = void>
+struct __is_invalid_std_hash
+ : integral_constant<bool, __is_hash_specialization<_Hash>::value> {};
+
+template <class _Arg>
+struct __is_invalid_std_hash<hash<_Arg>,
+ typename __void_t<integral_constant<size_t, sizeof(hash<_Arg>)>>::type
+> : false_type {};
+
+
+#endif
+
+template <class _Key, class _Hash, bool _EnableAssertions = false,
+ bool _IsStdHashInvalid = __is_invalid_std_hash<_Hash>::value,
+ class = typename enable_if<_IsStdHashInvalid>::type>
+constexpr bool __check_hash_requirements() {
+ static_assert(!_IsStdHashInvalid || !_EnableAssertions,
+ "no matching specialization for std::hash<Key>");
+ return false;
+}
+
+template <class _Key, class _Hash, bool _EnableAssertions = false,
+ class _StdHashIsInvalid = __is_invalid_std_hash<_Hash>,
+ class = typename enable_if<!_StdHashIsInvalid::value>::type,
+ bool _IsCopyConstructible = is_copy_constructible<_Hash>::value,
+ bool _IsMoveConstructible = is_move_constructible<_Hash>::value,
+ bool _HasCallOperator = __invokable<_Hash, _Key const&>::value,
+ bool _HasCorrectReturnType = __invokable_r<size_t, _Hash, _Key const&>::value,
+ bool _IsGood = _IsCopyConstructible
+ && _IsMoveConstructible && _HasCallOperator
+ && _HasCorrectReturnType>
+constexpr bool __check_hash_requirements() {
+ static_assert(_IsCopyConstructible || !_EnableAssertions,
+ "the specified hash is required to be copy constructible");
+ static_assert(_IsMoveConstructible || !_IsCopyConstructible || !_EnableAssertions,
+ "the specified hash is required to be copy and move constructible");
+ static_assert(_HasCallOperator || !_EnableAssertions,
+ "the specified hash is required to be callable type");
+ static_assert(_HasCorrectReturnType || !_HasCallOperator || !_EnableAssertions,
+ "the specified hash is required to return an type convertible to size_t");
+
+ return _IsGood;
+}
+
+
template <class _Key, class _Hash>
-using __check_hash_requirements = integral_constant<bool,
- is_copy_constructible<_Hash>::value &&
- is_move_constructible<_Hash>::value &&
- __invokable_r<size_t, _Hash, _Key const&>::value
->;
+constexpr
+typename enable_if<_VSTD::__check_hash_requirements<_Key, _Hash>(), bool>::type
+__diagnose_hash_requirements_and_warnings()
+ _LIBCPP_DIAGNOSE_WARNING(!__invokable<_Hash const&, _Key const&>::value,
+ "the specified hash does not provide a const call operator")
+{
+ return true;
+}
+
+template <class _Key, class _Hash>
+constexpr
+typename enable_if<!_VSTD::__check_hash_requirements<_Key, _Hash>(), bool>::type
+__diagnose_hash_requirements_and_warnings()
+{
+ return _VSTD::__check_hash_requirements<_Key, _Hash, /*EnableAssertions*/true>()
+ || true;
+}
+
template <class _Key, class _Hash = std::hash<_Key> >
using __has_enabled_hash = integral_constant<bool,
- __check_hash_requirements<_Key, _Hash>::value &&
+ __check_hash_requirements<_Key, _Hash>() &&
is_default_constructible<_Hash>::value
>;
Index: include/__hash_table
===================================================================
--- include/__hash_table
+++ include/__hash_table
@@ -871,18 +871,14 @@
template <class _Key, class _Hash, class _Equal, class _Alloc>
struct __diagnose_hash_table_helper {
static constexpr bool __trigger_diagnostics()
- _LIBCPP_DIAGNOSE_WARNING(__check_hash_requirements<_Key, _Hash>::value
- && !__invokable<_Hash const&, _Key const&>::value,
- "the specified hash functor does not provide a const call operator")
_LIBCPP_DIAGNOSE_WARNING(is_copy_constructible<_Equal>::value
&& !__invokable<_Equal const&, _Key const&, _Key const&>::value,
"the specified comparator type does not provide a const call operator")
{
- static_assert(__check_hash_requirements<_Key, _Hash>::value,
- "the specified hash does not meet the Hash requirements");
static_assert(is_copy_constructible<_Equal>::value,
"the specified comparator is required to be copy constructible");
- return true;
+ return _VSTD::__diagnose_hash_requirements_and_warnings<_Key, _Hash>()
+ || true;
}
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
