diff mbox series

[committed] libstdc++: fix is_default_constructible for hash containers [PR 100863]

Message ID YPbrHCl1vCM5GsHK@redhat.com
State New
Headers show
Series [committed] libstdc++: fix is_default_constructible for hash containers [PR 100863] | expand

Commit Message

Jonathan Wakely July 20, 2021, 3:26 p.m. UTC
On 02/06/21 13:35 +0100, Jonathan Wakely wrote:
>The allocator, hash function and equality function should all be
>value-initialized by the default constructor of an unordered container.
>Do it in the EBO helper, so we don't have to get it right in multiple
>places.
>
>Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
>
>libstdc++-v3/ChangeLog:
>
>	PR libstdc++/100863
>	PR libstdc++/65816
>	* include/bits/hashtable_policy.h (_Hashtable_ebo_helper):
>	Value-initialize subobject.
>	* testsuite/23_containers/unordered_map/allocator/default_init.cc:
>	Remove XFAIL.
>	* testsuite/23_containers/unordered_set/allocator/default_init.cc:
>	Remove XFAIL.


The recent change to _Hashtable_ebo_helper for this PR broke the
is_default_constructible trait for a hash container with a non-default
constructible allocator. That happens because the constructor needs to
be user-provided in order to initialize the member, and so is not
defined as deleted when the type is not default constructible.

By making _Hashtable derive from _Enable_special_members we can ensure
that the default constructor for the std::unordered_xxx containers is
deleted when it would be ill-formed. This makes the trait give the
correct answer.

Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

libstdc++-v3/ChangeLog:

         PR libstdc++/100863
         * include/bits/hashtable.h (_Hashtable): Conditionally delete
         default constructor by deriving from _Enable_special_members.
         * testsuite/23_containers/unordered_map/cons/default.cc: New test.
         * testsuite/23_containers/unordered_set/cons/default.cc: New test.


Tested powerpc64le-linux. Committed to trunk.
diff mbox series

Patch

commit 89ec3b67dbe856a447d068b053bc19559f136f43
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jul 20 15:20:41 2021

    libstdc++: fix is_default_constructible for hash containers [PR 100863]
    
    The recent change to _Hashtable_ebo_helper for this PR broke the
    is_default_constructible trait for a hash container with a non-default
    constructible allocator. That happens because the constructor needs to
    be user-provided in order to initialize the member, and so is not
    defined as deleted when the type is not default constructible.
    
    By making _Hashtable derive from _Enable_special_members we can ensure
    that the default constructor for the std::unordered_xxx containers is
    deleted when it would be ill-formed. This makes the trait give the
    correct answer.
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/100863
            * include/bits/hashtable.h (_Hashtable): Conditionally delete
            default constructor by deriving from _Enable_special_members.
            * testsuite/23_containers/unordered_map/cons/default.cc: New test.
            * testsuite/23_containers/unordered_set/cons/default.cc: New test.

diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h
index dfc2a2a7800..adb59213f2d 100644
--- a/libstdc++-v3/include/bits/hashtable.h
+++ b/libstdc++-v3/include/bits/hashtable.h
@@ -33,6 +33,7 @@ 
 #pragma GCC system_header
 
 #include <bits/hashtable_policy.h>
+#include <bits/enable_special_members.h>
 #if __cplusplus > 201402L
 # include <bits/node_handle.h>
 #endif
@@ -48,6 +49,17 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 		       // Mandatory to have erase not throwing.
 		       __is_nothrow_invocable<const _Hash&, const _Tp&>>>;
 
+  // Helper to conditionally delete the default constructor.
+  // The _Hash_node_base type is used to distinguish this specialization
+  // from any other potentially-overlapping subobjects of the hashtable.
+  template<typename _Equal, typename _Hash, typename _Allocator>
+    using _Hashtable_enable_default_ctor
+      = _Enable_special_members<__and_<is_default_constructible<_Equal>,
+				       is_default_constructible<_Hash>,
+				       is_default_constructible<_Allocator>>{},
+				true, true, true, true, true,
+				__detail::_Hash_node_base>;
+
   /**
    *  Primary class template _Hashtable.
    *
@@ -183,7 +195,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       private __detail::_Hashtable_alloc<
 	__alloc_rebind<_Alloc,
 		       __detail::_Hash_node<_Value,
-					    _Traits::__hash_cached::value>>>
+					    _Traits::__hash_cached::value>>>,
+      private _Hashtable_enable_default_ctor<_Equal, _Hash, _Alloc>
     {
       static_assert(is_same<typename remove_cv<_Value>::type, _Value>::value,
 	  "unordered container must have a non-const, non-volatile value_type");
diff --git a/libstdc++-v3/testsuite/23_containers/unordered_map/cons/default.cc b/libstdc++-v3/testsuite/23_containers/unordered_map/cons/default.cc
new file mode 100644
index 00000000000..e4f836fde3e
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/unordered_map/cons/default.cc
@@ -0,0 +1,33 @@ 
+// { dg-do compile { target c++11 } }
+#include <unordered_map>
+
+static_assert( std::is_default_constructible<std::unordered_map<int, int>>{}, "" );
+
+template<typename T>
+  struct NoDefaultConsAlloc
+  {
+    using value_type = T;
+
+    NoDefaultConsAlloc(int) noexcept { }
+
+    template<typename U>
+      NoDefaultConsAlloc(const NoDefaultConsAlloc<U>&) { }
+
+    T *allocate(std::size_t n)
+    { return std::allocator<T>().allocate(n); }
+
+    void deallocate(T *p, std::size_t n)
+    { std::allocator<T>().deallocate(p, n); }
+  };
+
+using Map = std::unordered_map<int, int, std::hash<int>, std::equal_to<int>,
+			       NoDefaultConsAlloc<std::pair<const int, int>>>;
+static_assert( ! std::is_default_constructible<Map>{}, "PR libstdc++/100863" );
+
+struct Hash : std::hash<int> { Hash(int) { } };
+using Map2 = std::unordered_map<int, int, Hash>;
+static_assert( ! std::is_default_constructible<Map2>{}, "PR libstdc++/100863" );
+
+struct Equal : std::equal_to<int> { Equal(int) { } };
+using Map3 = std::unordered_map<int, int, std::hash<int>, Equal>;
+static_assert( ! std::is_default_constructible<Map3>{}, "PR libstdc++/100863" );
diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/cons/default.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/cons/default.cc
new file mode 100644
index 00000000000..42fbf3d7997
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/unordered_set/cons/default.cc
@@ -0,0 +1,33 @@ 
+// { dg-do compile { target c++11 } }
+#include <unordered_set>
+
+static_assert( std::is_default_constructible<std::unordered_set<int>>{}, "" );
+
+template<typename T>
+  struct NoDefaultConsAlloc
+  {
+    using value_type = T;
+
+    NoDefaultConsAlloc(int) noexcept { }
+
+    template<typename U>
+      NoDefaultConsAlloc(const NoDefaultConsAlloc<U>&) { }
+
+    T *allocate(std::size_t n)
+    { return std::allocator<T>().allocate(n); }
+
+    void deallocate(T *p, std::size_t n)
+    { std::allocator<T>().deallocate(p, n); }
+  };
+
+using Set = std::unordered_set<int, std::hash<int>, std::equal_to<int>,
+			       NoDefaultConsAlloc<int>>;
+static_assert( ! std::is_default_constructible<Set>{}, "PR libstdc++/100863" );
+
+struct Hash : std::hash<int> { Hash(int) { } };
+using Set2 = std::unordered_set<int, Hash>;
+static_assert( ! std::is_default_constructible<Set2>{}, "PR libstdc++/100863" );
+
+struct Equal : std::equal_to<int> { Equal(int) { } };
+using Set3 = std::unordered_set<int, std::hash<int>, Equal>;
+static_assert( ! std::is_default_constructible<Set3>{}, "PR libstdc++/100863" );