diff options
author | Jonathan Wakely <jwakely@redhat.com> | 2024-11-01 12:49:53 +0000 |
---|---|---|
committer | Jonathan Wakely <redi@gcc.gnu.org> | 2024-11-13 20:21:39 +0000 |
commit | afc9351ebbac1ed55d7c472af817b172bdb662e6 (patch) | |
tree | 6797dd7a24eb84b8da3a9b351313b9803bd83b36 | |
parent | 9fcbbb3d104717447cffb65f6ef000969a7b7bb4 (diff) | |
download | gcc-afc9351ebbac1ed55d7c472af817b172bdb662e6.zip gcc-afc9351ebbac1ed55d7c472af817b172bdb662e6.tar.gz gcc-afc9351ebbac1ed55d7c472af817b172bdb662e6.tar.bz2 |
libstdc++: Allow unordered_set assignment to assign to existing nodes
Currently the _ReuseOrAllocNode::operator(Args&&...) function always
destroys the value stored in recycled nodes and constructs a new value.
The _ReuseOrAllocNode type is only ever used for implementing
assignment, either from another unordered container of the same type, or
from std::initializer_list<value_type>. Consequently, the parameter pack
Args only ever consists of a single parameter or type const value_type&
or value_type. We can replace the variadic parameter pack with a single
forwarding reference parameter, and when the value_type is assignable
from that type we can use assignment instead of destroying the existing
value and then constructing a new one.
Using assignment is typically only possible for sets, because for maps
the value_type is std::pair<const key_type, mapped_type> and in most
cases std::is_assignable_v<const key_type&, const key_type&> is false.
libstdc++-v3/ChangeLog:
* include/bits/hashtable_policy.h (_ReuseOrAllocNode::operator()):
Replace parameter pack with a single parameter. Assign to
existing value when possible.
* testsuite/23_containers/unordered_multiset/allocator/move_assign.cc:
Adjust expected count of operations.
* testsuite/23_containers/unordered_set/allocator/move_assign.cc:
Likewise.
Reviewed-by: François Dumont <fdumont@gcc.gnu.org>
3 files changed, 35 insertions, 17 deletions
diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h index b5f837e..7a3c66c 100644 --- a/libstdc++-v3/include/bits/hashtable_policy.h +++ b/libstdc++-v3/include/bits/hashtable_policy.h @@ -172,24 +172,39 @@ namespace __detail ~_ReuseOrAllocNode() { _M_h._M_deallocate_nodes(_M_nodes); } - template<typename... _Args> +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr + template<typename _Arg> __node_ptr - operator()(_Args&&... __args) + operator()(_Arg&& __arg) { if (!_M_nodes) - return _M_h._M_allocate_node(std::forward<_Args>(__args)...); + return _M_h._M_allocate_node(std::forward<_Arg>(__arg)); + + using value_type = typename _NodeAlloc::value_type::value_type; __node_ptr __node = _M_nodes; - _M_nodes = _M_nodes->_M_next(); - __node->_M_nxt = nullptr; - auto& __a = _M_h._M_node_allocator(); - __node_alloc_traits::destroy(__a, __node->_M_valptr()); - _NodePtrGuard<__hashtable_alloc, __node_ptr> __guard { _M_h, __node }; - __node_alloc_traits::construct(__a, __node->_M_valptr(), - std::forward<_Args>(__args)...); - __guard._M_ptr = nullptr; + if constexpr (is_assignable<value_type&, _Arg>::value) + { + __node->_M_v() = std::forward<_Arg>(__arg); + _M_nodes = _M_nodes->_M_next(); + __node->_M_nxt = nullptr; + } + else + { + _M_nodes = _M_nodes->_M_next(); + __node->_M_nxt = nullptr; + auto& __a = _M_h._M_node_allocator(); + __node_alloc_traits::destroy(__a, __node->_M_valptr()); + _NodePtrGuard<__hashtable_alloc, __node_ptr> + __guard{ _M_h, __node }; + __node_alloc_traits::construct(__a, __node->_M_valptr(), + std::forward<_Arg>(__arg)); + __guard._M_ptr = nullptr; + } return __node; } +#pragma GCC diagnostic pop private: __node_ptr _M_nodes; diff --git a/libstdc++-v3/testsuite/23_containers/unordered_multiset/allocator/move_assign.cc b/libstdc++-v3/testsuite/23_containers/unordered_multiset/allocator/move_assign.cc index 50608ec..6d00354 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_multiset/allocator/move_assign.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_multiset/allocator/move_assign.cc @@ -46,8 +46,9 @@ void test01() VERIFY( 1 == v1.get_allocator().get_personality() ); VERIFY( 2 == v2.get_allocator().get_personality() ); - VERIFY( counter_type::move_count == 1 ); - VERIFY( counter_type::destructor_count == 2 ); + VERIFY( counter_type::move_count == 0 ); + // 1 element in v1 destroyed. + VERIFY( counter_type::destructor_count == 1 ); } void test02() diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/move_assign.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/move_assign.cc index 677ea67..6be7002 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/move_assign.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/move_assign.cc @@ -52,8 +52,9 @@ void test01() VERIFY( 1 == v1.get_allocator().get_personality() ); VERIFY( 2 == v2.get_allocator().get_personality() ); - VERIFY( counter_type::move_count == 1 ); - VERIFY( counter_type::destructor_count == 2 ); + VERIFY( counter_type::move_count == 0 ); + // 1 element in v1 destroyed. + VERIFY( counter_type::destructor_count == 1 ); } // Check there's nothing left allocated or constructed. @@ -130,8 +131,9 @@ void test03() VERIFY( 1 == v1.get_allocator().get_personality() ); VERIFY( 2 == v2.get_allocator().get_personality() ); - VERIFY( counter_type::move_count == 1 ); - VERIFY( counter_type::destructor_count == i + 1 ); + VERIFY( counter_type::move_count == 0 ); + // (i - 1) elements in v2 destroyed, and 1 element in v1 destroyed. + VERIFY( counter_type::destructor_count == i ); } // Check there's nothing left allocated or constructed. |