[libc++] Attempt to improve diagnostics about Hash requirement violations

[EricWF]

Summary

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?

include/__hash_table

public:
     template <class> friend class __hash_map_node_destructor;
 };
 
 
 #ifndef _LIBCPP_CXX03_LANG
 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;
   }
 };
 
 template <class _Key, class _Value, class _Hash, class _Equal, class _Alloc>
 struct __diagnose_hash_table_helper<
   __hash_value_type<_Key, _Value>,
   __unordered_map_hasher<_Key, __hash_value_type<_Key, _Value>, _Hash>,
   __unordered_map_equal<_Key, __hash_value_type<_Key, _Value>, _Equal>,

include/utility

     size_t operator()(_Tp __v) const _NOEXCEPT
     {
         typedef typename underlying_type<_Tp>::type type;
         return hash<type>{}(static_cast<type>(__v));
     }
 };
 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;
 };
 
 template <class _Tp>
 struct _LIBCPP_TEMPLATE_VIS hash : public __enum_hash<_Tp>
 {

   _LIBCPP_INLINE_VISIBILITY
   size_t operator()(nullptr_t) const _NOEXCEPT {
     return 662607004ull;
   }
 };
 #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
 >;
 
 #if _LIBCPP_STD_VER > 14
 template <class _Type, class>
 using __enable_hash_helper_imp = _Type;
 
 template <class _Type, class ..._Keys>

test/libcxx/containers/unord/unord.set/missing_hash_specialization.fail.cpp

 //===----------------------------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 // 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 {
   BadHashNoCopy() = default;
   BadHashNoCopy(BadHashNoCopy const&) = delete;
 
   template <class T>
   size_t operator()(T const&) const { return 0; }
struct GoodHashNoDefault {
   template <class T>
   size_t operator()(T const&) const { return 0; }
 };
 
 int main() {
 
   {
     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>;
     Set s(/*bucketcount*/42, GoodHashNoDefault(nullptr));
   }
 }