From: Jonathan Wakely <jwakely@redhat.com>
To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org
Subject: [committed] libstdc++: Fix non-default constructors for hash containers [PR101583]
Date: Thu, 22 Jul 2021 19:41:41 +0100 [thread overview]
Message-ID: <YPm75Yxooq7q75bs@redhat.com> (raw)
In-Reply-To: <YPbrHCl1vCM5GsHK@redhat.com>
[-- 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};
prev parent reply other threads:[~2021-07-22 18:41 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` Jonathan Wakely [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YPm75Yxooq7q75bs@redhat.com \
--to=jwakely@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=libstdc++@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).