* [committed] libstdc++: Value-initialize objects held by EBO helpers [PR 100863] @ 2021-06-02 12:35 Jonathan Wakely 2021-07-20 15:26 ` [committed] libstdc++: fix is_default_constructible for hash containers " Jonathan Wakely 0 siblings, 1 reply; 3+ messages in thread From: Jonathan Wakely @ 2021-06-02 12:35 UTC (permalink / raw) To: libstdc++, gcc-patches [-- Attachment #1: Type: text/plain, Size: 638 bytes --] 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. Tested powerpc64le-linux. Committed to trunk. [-- Attachment #2: patch.txt --] [-- Type: text/plain, Size: 3244 bytes --] commit f8f0193b5b83f6e85d65015e79c803295baf5166 Author: Jonathan Wakely <jwakely@redhat.com> Date: Wed Jun 2 12:34:48 2021 libstdc++: Value-initialize objects held by EBO helpers [PR 100863] 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. diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h index 1090a398e1e..2130c958262 100644 --- a/libstdc++-v3/include/bits/hashtable_policy.h +++ b/libstdc++-v3/include/bits/hashtable_policy.h @@ -1162,7 +1162,7 @@ namespace __detail struct _Hashtable_ebo_helper<_Nm, _Tp, true> : private _Tp { - _Hashtable_ebo_helper() = default; + _Hashtable_ebo_helper() noexcept(noexcept(_Tp())) : _Tp() { } template<typename _OtherTp> _Hashtable_ebo_helper(_OtherTp&& __tp) @@ -1188,7 +1188,7 @@ namespace __detail _Tp& _M_get() { return _M_tp; } private: - _Tp _M_tp; + _Tp _M_tp{}; }; /** @@ -1246,6 +1246,7 @@ namespace __detail // We need the default constructor for the local iterators and _Hashtable // default constructor. _Hash_code_base() = default; + _Hash_code_base(const _Hash& __hash) : __ebo_hash(__hash) { } __hash_code @@ -1639,6 +1640,7 @@ namespace __detail protected: _Hashtable_base() = default; + _Hashtable_base(const _Hash& __hash, const _Equal& __eq) : __hash_code_base(__hash), _EqualEBO(__eq) { } diff --git a/libstdc++-v3/testsuite/23_containers/unordered_map/allocator/default_init.cc b/libstdc++-v3/testsuite/23_containers/unordered_map/allocator/default_init.cc index bbfd683d433..12f1163951e 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_map/allocator/default_init.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_map/allocator/default_init.cc @@ -17,7 +17,6 @@ // { dg-do run { target c++11 } } // { dg-options "-O0" } -// { dg-xfail-run-if "PR c++/65816" { *-*-* } } #include <unordered_map> #include <testsuite_hooks.h> diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/default_init.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/default_init.cc index 6ee32d45ceb..1ea6603024e 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/default_init.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/default_init.cc @@ -17,7 +17,6 @@ // { dg-do run { target c++11 } } // { dg-options "-O0" } -// { dg-xfail-run-if "PR c++/65816" { *-*-* } } #include <unordered_set> #include <testsuite_hooks.h> ^ permalink raw reply [flat|nested] 3+ messages in thread
* [committed] libstdc++: fix is_default_constructible for hash containers [PR 100863] 2021-06-02 12:35 [committed] libstdc++: Value-initialize objects held by EBO helpers [PR 100863] Jonathan Wakely @ 2021-07-20 15:26 ` Jonathan Wakely 2021-07-22 18:41 ` [committed] libstdc++: Fix non-default constructors for hash containers [PR101583] Jonathan Wakely 0 siblings, 1 reply; 3+ messages in thread From: Jonathan Wakely @ 2021-07-20 15:26 UTC (permalink / raw) To: libstdc++, gcc-patches [-- Attachment #1: Type: text/plain, Size: 1670 bytes --] 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. [-- Attachment #2: patch.txt --] [-- Type: text/x-patch, Size: 5799 bytes --] 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" ); ^ permalink raw reply [flat|nested] 3+ messages in thread
* [committed] libstdc++: Fix non-default constructors for hash containers [PR101583] 2021-07-20 15:26 ` [committed] libstdc++: fix is_default_constructible for hash containers " Jonathan Wakely @ 2021-07-22 18:41 ` Jonathan Wakely 0 siblings, 0 replies; 3+ messages in thread From: Jonathan Wakely @ 2021-07-22 18:41 UTC (permalink / raw) To: libstdc++, gcc-patches [-- Attachment #1: Type: text/plain, Size: 2430 bytes --] 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 <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. > 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. [-- Attachment #2: patch.txt --] [-- Type: text/x-patch, Size: 6199 bytes --] commit 8ed6cfbbee74ec9e03f2558b9c36f61dd7d4dcfd Author: Jonathan Wakely <jwakely@redhat.com> 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<typename _Equal, typename _Hash, typename _Allocator> using _Hashtable_enable_default_ctor - = _Enable_special_members<__and_<is_default_constructible<_Equal>, + = _Enable_default_constructor<__and_<is_default_constructible<_Equal>, 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<bool _No_realloc = true> @@ -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<typename _InputIterator> @@ -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<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" ); + +// PR libstdc++/101583 +// verify non-default ctors can still be used +using Map4 = std::unordered_map<int, int, Hash, Equal, + NoDefaultConsAlloc<std::pair<const int, int>>>; +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<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" ); + +// PR libstdc++/101583 +// verify non-default ctors can still be used +using Set4 = std::unordered_set<int, Hash, Equal, NoDefaultConsAlloc<int>>; +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}; ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-07-22 18:41 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-02 12:35 [committed] libstdc++: Value-initialize objects held by EBO helpers [PR 100863] Jonathan Wakely 2021-07-20 15:26 ` [committed] libstdc++: fix is_default_constructible for hash containers " Jonathan Wakely 2021-07-22 18:41 ` [committed] libstdc++: Fix non-default constructors for hash containers [PR101583] Jonathan Wakely
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).