On 07/11/19 20:28 +0100, François Dumont wrote:
From what I understood from recent fix the unordered containers need
to be updated the same way.
I hope you'll appreciate the usage of rvalue forwarding. Containers
Yes, I think it makes sense.
node values are moved as soon as _M_assign is called with a rvalue
reference to the source container.
Additionnaly this patch removes usages of lambdas in _Hashtable.
If you confirm it I'll check for the same on _Rb_tree.
* include/bits/hashtable.h (_Hashtable<>::__alloc_node_gen_t): New
template alias.
(_Hashtable<>::__mv_if_value_type_mv_noexcept): New.
(_Hashtable<>::__fwd_value): New.
(_Hashtable<>::_M_assign_elements<>): Remove _NodeGenerator template
parameter.
(_Hashtable<>::_M_assign<>): Add _Ht template parameter.
(_Hashtable<>::operator=(const _Hashtable<>&)): Adapt.
(_Hashtable<>::_M_move_assign): Adapt.
(_Hashtable<>::_Hashtable(const _Hashtable&)): Adapt.
(_Hashtable<>::_Hashtable(const _Hashtable&, const allocator_type&)):
Adapt.
(_Hashtable<>::_Hashtable(_Hashtable&&, const allocator_type&)):
Adapt.
* testsuite/23_containers/unordered_set/92124.cc: New.
Tested under Linux x86_64.
Ok to commit ?
François
diff --git a/libstdc++-v3/include/bits/hashtable.h
b/libstdc++-v3/include/bits/hashtable.h
index ab579a7059e..c2b2219d471 100644
--- a/libstdc++-v3/include/bits/hashtable.h
+++ b/libstdc++-v3/include/bits/hashtable.h
@@ -255,6 +255,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
using __reuse_or_alloc_node_gen_t =
__detail::_ReuseOrAllocNode<__node_alloc_type>;
+ using __alloc_node_gen_t =
+ __detail::_AllocNode<__node_alloc_type>;
// Simple RAII type for managing a node containing an element
struct _Scoped_node
@@ -280,6 +282,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__node_type* _M_node;
};
+ template<typename _Tp>
+ static constexpr
+ typename conditional<__move_if_noexcept_cond<value_type>::value,
+ const _Tp&, _Tp&&>::type
+ __mv_if_value_type_mv_noexcept(_Tp& __x) noexcept
+ { return std::move(__x); }
This is only used in one place. Adding a function doesn't seem
worthwhile, you can just do this where you use it:
using _Fwd_Ht = typename
conditional<__move_if_noexcept_cond<value_type>::value,
const _Ht&, _Ht>::type;
_M_assign(std::forward<_Fwd_Ht>(__ht), __alloc_gen);
+ template<typename _Ht>
+ static constexpr
+ typename conditional<!std::is_lvalue_reference<_Ht>::value,
+ value_type&&, const value_type&>::type
I think I'd prefer to reverse the condition, i.e.
typename conditional<is_lvalue_reference<_Ht>::value,
const value_type&, value_type&&>::type
+ __fwd_value(_Ht&&, value_type& __val) noexcept
+ { return std::move(__val); }
Since this doesn't use the first parameter, it can just be removed:
template<typename _Ht>
static constexpr
typename conditional<std::is_lvalue_reference<_Ht>::value,
const value_type&, value_type&&>::type
__fwd_value(value_type& __val) noexcept
{ return std::move(__val); }
That simplifies the usage from:
__fwd_value(std::forward<_Ht>(__ht), __ht_n->_M_v()))
so it becomes:
__fwd_value<_Ht>(__ht_n->_M_v()))
Maybe __fwd_value<_Ht> should be __fwd_value_for<_Ht> so it's clear
how it depends on _Ht (because otherwise it looks like it is
forwarding as _Ht&& like std::forward<_Ht> would).
What do you think?
