From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 12258385042D for ; Thu, 22 Jul 2021 18:41:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 12258385042D Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-242-8XjTm02HPXu5F-DBirEi-g-1; Thu, 22 Jul 2021 14:41:44 -0400 X-MC-Unique: 8XjTm02HPXu5F-DBirEi-g-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 28198100CF7F; Thu, 22 Jul 2021 18:41:43 +0000 (UTC) Received: from localhost (unknown [10.33.36.7]) by smtp.corp.redhat.com (Postfix) with ESMTP id C4E515B826; Thu, 22 Jul 2021 18:41:42 +0000 (UTC) Date: Thu, 22 Jul 2021 19:41:41 +0100 From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [committed] libstdc++: Fix non-default constructors for hash containers [PR101583] Message-ID: References: MIME-Version: 1.0 In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/mixed; boundary="cr/RqSkqXw1meNZC" Content-Disposition: inline X-Spam-Status: No, score=-14.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=unavailable autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Jul 2021 18:41:49 -0000 --cr/RqSkqXw1meNZC Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline On 20/07/21 16:26 +0100, Jonathan Wakely wrote: >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 >> >>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 > >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. > When I added the new mixin to _Hashtable, I forgot to explicitly construct it in each non-default constructor. That means you can't use any constructors unless all three of the hash function, equality function, and allocator are all default constructible. PR libstdc++/101583 * include/bits/hashtable.h (_Hashtable): Replace mixin with _Enable_default_ctor. Construct it explicitly in all non-forwarding, non-defaulted constructors. * testsuite/23_containers/unordered_map/cons/default.cc: Check non-default constructors can be used. * testsuite/23_containers/unordered_set/cons/default.cc: Likewise. Tested powerpc64le-linux. Committed to trunk. --cr/RqSkqXw1meNZC Content-Type: text/x-patch; charset=us-ascii Content-Disposition: attachment; filename="patch.txt" commit 8ed6cfbbee74ec9e03f2558b9c36f61dd7d4dcfd Author: Jonathan Wakely Date: Thu Jul 22 18:49:57 2021 libstdc++: Fix non-default constructors for hash containers [PR101583] When I added the new mixin to _Hashtable, I forgot to explicitly construct it in each non-default constructor. That means you can't use any constructors unless all three of the hash function, equality function, and allocator are all default constructible. libstdc++-v3/ChangeLog: PR libstdc++/101583 * include/bits/hashtable.h (_Hashtable): Replace mixin with _Enable_default_ctor. Construct it explicitly in all non-forwarding, non-defaulted constructors. * testsuite/23_containers/unordered_map/cons/default.cc: Check non-default constructors can be used. * testsuite/23_containers/unordered_set/cons/default.cc: Likewise. diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h index adb59213f2d..92516b81ae5 100644 --- a/libstdc++-v3/include/bits/hashtable.h +++ b/libstdc++-v3/include/bits/hashtable.h @@ -54,11 +54,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // from any other potentially-overlapping subobjects of the hashtable. template using _Hashtable_enable_default_ctor - = _Enable_special_members<__and_, + = _Enable_default_constructor<__and_, is_default_constructible<_Hash>, is_default_constructible<_Allocator>>{}, - true, true, true, true, true, - __detail::_Hash_node_base>; + __detail::_Hash_node_base>; /** * Primary class template _Hashtable. @@ -228,6 +227,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Equal, _Hash, _RangeHash, _Unused, _RehashPolicy, _Traits>; + using __enable_default_ctor + = _Hashtable_enable_default_ctor<_Equal, _Hash, _Alloc>; public: typedef _Key key_type; @@ -484,7 +485,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Hashtable(const _Hash& __h, const _Equal& __eq, const allocator_type& __a) : __hashtable_base(__h, __eq), - __hashtable_alloc(__node_alloc_type(__a)) + __hashtable_alloc(__node_alloc_type(__a)), + __enable_default_ctor(_Enable_default_constructor_tag{}) { } template @@ -551,7 +553,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION explicit _Hashtable(const allocator_type& __a) - : __hashtable_alloc(__node_alloc_type(__a)) + : __hashtable_alloc(__node_alloc_type(__a)), + __enable_default_ctor(_Enable_default_constructor_tag{}) { } template @@ -1435,6 +1438,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __rehash_base(__ht), __hashtable_alloc( __node_alloc_traits::_S_select_on_copy(__ht._M_node_allocator())), + __enable_default_ctor(__ht), _M_buckets(nullptr), _M_bucket_count(__ht._M_bucket_count), _M_element_count(__ht._M_element_count), @@ -1457,6 +1461,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __map_base(__ht), __rehash_base(__ht), __hashtable_alloc(std::move(__a)), + __enable_default_ctor(__ht), _M_buckets(__ht._M_buckets), _M_bucket_count(__ht._M_bucket_count), _M_before_begin(__ht._M_before_begin._M_nxt), @@ -1487,6 +1492,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __map_base(__ht), __rehash_base(__ht), __hashtable_alloc(__node_alloc_type(__a)), + __enable_default_ctor(__ht), _M_buckets(), _M_bucket_count(__ht._M_bucket_count), _M_element_count(__ht._M_element_count), @@ -1508,6 +1514,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __map_base(__ht), __rehash_base(__ht), __hashtable_alloc(std::move(__a)), + __enable_default_ctor(__ht), _M_buckets(nullptr), _M_bucket_count(__ht._M_bucket_count), _M_element_count(__ht._M_element_count), diff --git a/libstdc++-v3/testsuite/23_containers/unordered_map/cons/default.cc b/libstdc++-v3/testsuite/23_containers/unordered_map/cons/default.cc index e4f836fde3e..d64d078a7da 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_map/cons/default.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_map/cons/default.cc @@ -31,3 +31,18 @@ static_assert( ! std::is_default_constructible{}, "PR libstdc++/100863" ); struct Equal : std::equal_to { Equal(int) { } }; using Map3 = std::unordered_map, Equal>; static_assert( ! std::is_default_constructible{}, "PR libstdc++/100863" ); + +// PR libstdc++/101583 +// verify non-default ctors can still be used +using Map4 = std::unordered_map>>; +Hash h(1); +Equal eq(1); +Map4::allocator_type a(1); +Map4 m{1, h, eq, a}; +Map4 m2{m.begin(), m.end(), m.size(), h, eq, a}; +Map4 m3{{{1,1}, {2,2}, {3,3}}, 3, h, eq, a}; +Map4 m4{m}; +Map4 m5{m, a}; +Map4 m6{std::move(m)}; +Map4 m7{std::move(m6), a}; diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/cons/default.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/cons/default.cc index 42fbf3d7997..41281d3d774 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_set/cons/default.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_set/cons/default.cc @@ -31,3 +31,17 @@ static_assert( ! std::is_default_constructible{}, "PR libstdc++/100863" ); struct Equal : std::equal_to { Equal(int) { } }; using Set3 = std::unordered_set, Equal>; static_assert( ! std::is_default_constructible{}, "PR libstdc++/100863" ); + +// PR libstdc++/101583 +// verify non-default ctors can still be used +using Set4 = std::unordered_set>; +Hash h(1); +Equal eq(1); +Set4::allocator_type a(1); +Set4 s{1, h, eq, a}; +Set4 s2{s.begin(), s.end(), s.size(), h, eq, a}; +Set4 s3{{1, 2, 3}, 3, h, eq, a}; +Set4 s4{s}; +Set4 s5{s, a}; +Set4 s6{std::move(s)}; +Set4 s7{std::move(s6), a}; --cr/RqSkqXw1meNZC--