[libcxx] r296919 - Fix hash requirements check in __hash_table.

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 3 14:35:58 PST 2017


Author: ericwf
Date: Fri Mar  3 16:35:58 2017
New Revision: 296919

URL: http://llvm.org/viewvc/llvm-project?rev=296919&view=rev
Log:
Fix hash requirements check in __hash_table.

r296565 attempted to add better diagnostics when an unordered container
is instantiated with a hash that doesn't meet the Hash requirements.

However I mistakenly checked the wrong set of requirements. Specifically
it checked if the hash met the requirements for specializations of
std::hash. However these requirements are stricter than the generic
Hash requirements.

This patch fixes the assertions to only check the Hash requirements.

Modified:
    libcxx/trunk/include/__hash_table
    libcxx/trunk/include/utility
    libcxx/trunk/test/libcxx/containers/unord/unord.set/missing_hash_specialization.fail.cpp

Modified: libcxx/trunk/include/__hash_table
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__hash_table?rev=296919&r1=296918&r2=296919&view=diff
==============================================================================
--- libcxx/trunk/include/__hash_table (original)
+++ libcxx/trunk/include/__hash_table Fri Mar  3 16:35:58 2017
@@ -871,16 +871,15 @@ public:
 template <class _Key, class _Hash, class _Equal, class _Alloc>
 struct __diagnose_hash_table_helper {
   static constexpr bool __trigger_diagnostics()
-    _LIBCPP_DIAGNOSE_WARNING(__has_enabled_hash<_Key, _Hash>::value
+    _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(__has_enabled_hash<_Key, _Hash>::value,
-      "the specified hash functor does not meet the requirements for an "
-      "enabled hash");
+    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;

Modified: libcxx/trunk/include/utility
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/utility?rev=296919&r1=296918&r2=296919&view=diff
==============================================================================
--- libcxx/trunk/include/utility (original)
+++ libcxx/trunk/include/utility Fri Mar  3 16:35:58 2017
@@ -1560,14 +1560,19 @@ struct _LIBCPP_TEMPLATE_VIS hash<nullptr
 #endif
 
 #ifndef _LIBCPP_CXX03_LANG
-template <class _Key, class _Hash = std::hash<_Key> >
-using __has_enabled_hash = integral_constant<bool,
-    is_default_constructible<_Hash>::value &&
+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
 >;
 
+template <class _Key, class _Hash = std::hash<_Key> >
+using __has_enabled_hash = integral_constant<bool,
+    __check_hash_requirements<_Key, _Hash>::value &&
+    is_default_constructible<_Hash>::value
+>;
+
 #if _LIBCPP_STD_VER > 14
 template <class _Type, class>
 using __enable_hash_helper_imp = _Type;

Modified: libcxx/trunk/test/libcxx/containers/unord/unord.set/missing_hash_specialization.fail.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/containers/unord/unord.set/missing_hash_specialization.fail.cpp?rev=296919&r1=296918&r2=296919&view=diff
==============================================================================
--- libcxx/trunk/test/libcxx/containers/unord/unord.set/missing_hash_specialization.fail.cpp (original)
+++ libcxx/trunk/test/libcxx/containers/unord/unord.set/missing_hash_specialization.fail.cpp Fri Mar  3 16:35:58 2017
@@ -23,14 +23,48 @@
 #include <utility>
 
 using VT = std::pair<int, int>;
-using Set = std::unordered_set<VT>;
+
+struct BadHashNoCopy {
+  BadHashNoCopy() = default;
+  BadHashNoCopy(BadHashNoCopy const&) = delete;
+
+  template <class T>
+  size_t operator()(T const&) const { return 0; }
+};
+
+struct BadHashNoCall {
+
+};
+
+
+struct GoodHashNoDefault {
+  explicit GoodHashNoDefault(void*) {}
+  template <class T>
+  size_t operator()(T const&) const { return 0; }
+};
 
 int main() {
 
-  Set s; // expected-error at __hash_table:* {{the specified hash functor does not meet the requirements for an enabled hash}}
+  {
+    using Set = std::unordered_set<VT>;
+    Set s; // expected-error at __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 at memory:* {{call to implicitly-deleted default constructor of 'std::__1::hash<std::__1::pair<int, int> >'}}
+  }
+  {
+    using Set = std::unordered_set<int, BadHashNoCopy>;
+    Set s; // expected-error at __hash_table:* {{the specified hash does not meet the Hash requirements}}
+  }
+  {
+    using Set = std::unordered_set<int, BadHashNoCall>;
+    Set s; // expected-error at __hash_table:* {{the specified hash does not meet the Hash requirements}}
+  }
+  {
+    using Set = std::unordered_set<int, GoodHashNoDefault>;
+    Set s(/*bucketcount*/42, GoodHashNoDefault(nullptr));
+  }
 }




More information about the cfe-commits mailing list