[PATCH] PR libstdc++/92124 on hashtable

[Jonathan Wakely]

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 
>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); }
>+
>+      template<typename _Ht>
>+	static constexpr
>+	typename conditional<!std::is_lvalue_reference<_Ht>::value,
>+			     value_type&&, const value_type&>::type
>+	__fwd_value(_Ht&&, value_type& __val) noexcept
>+	{ return std::move(__val); }
>+
>       // Metaprogramming for picking apart hash caching.
>       template<typename _Cond>
> 	using __if_hash_cached = __or_<__not_<__hash_cached>, _Cond>;
>@@ -406,13 +422,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 
>       // Assign *this using another _Hashtable instance. Either elements
>       // are copy or move depends on the _NodeGenerator.

N.B. I know this comment was already there, but could you please
change the second sentence to say:

"Whether elements are copied or moved ..."

>-      template<typename _Ht, typename _NodeGenerator>
>+      template<typename _Ht>
> 	void
>-	_M_assign_elements(_Ht&&, const _NodeGenerator&);
>+	_M_assign_elements(_Ht&&);
> 
>-      template<typename _NodeGenerator>
>+      template<typename _Ht, typename _NodeGenerator>
> 	void
>-	_M_assign(const _Hashtable&, const _NodeGenerator&);
>+	_M_assign(_Ht&&, const _NodeGenerator&);
> 
>       void
>       _M_move_assign(_Hashtable&&, true_type);