Re: [PATCH] Remove lambdas from _Rb_tree

[Jonathan Wakely <jwakely@redhat.com>]

On 20/11/20 08:17 +0100, François Dumont via Libstdc++ wrote:
>Here is what I am testing.
>
>I use your enum proposal as an alias for the bool type. I cannot use 
>it as template parameter on _M_copy unless I put it at std namespace 
>level to use it in definition of the outline _M_copy overload.

You can do it as a member, but the syntax is ugly:

   template<typename _Key, typename _Val, typename _KoV,
          typename _Compare, typename _Alloc>
     template<typename _Rb_tree<_Key, _Val, _KoV, _Compare, _Alloc>::_CopyType _MoveValues, typename _NodeGen>
       typename _Rb_tree<_Key, _Val, _KoV, _Compare, _Alloc>::_Link_type
       _Rb_tree<_Key, _Val, _KoV, _Compare, _Alloc>::
       _M_copy(_Link_type __x, _Base_ptr __p, _NodeGen& __node_gen)





>I also added some tests checking correct usage of 
>__move_if_noexcept_cond. I prefer not to change this condition as 
>proposed in this patch.

Yes, we can do that later if needed.

>I wonder if I am right to check moved values in those tests ?

Yes, I think that's good.

>I also wonder after writing those tests if we shouldn't clear the 
>moved instance, especially when values are moved ? I remember seeing 
>some discussion about this but I don't know the conclusion.

It's not required to clear them.

Leaving them with moved-from values means the memory isn't
deallocated, and those nodes can be reused if the container gets
assigned new values.

>    libstdc++: _Rb_tree code cleanup, remove lambdas
>
>    Use new template parameters to replace usage of lambdas to move or not
>    tree values on copy.
>
>    libstdc++-v3/ChangeLog:
>
>            * include/bits/move.h (_GLIBCXX_FWDREF): New.
>            * include/bits/stl_tree.h: Adapt to use latter.
>            (_Rb_tree<>::_M_clone_node): Add _MoveValue template parameter.
>            (_Rb_tree<>::_M_mbegin): New.
>            (_Rb_tree<>::_M_begin): Use latter.
>            (_Rb_tree<>::_M_copy): Add _MoveValues template parameter.
>            * testsuite/23_containers/map/allocator/move_cons.cc: New test.
>            * testsuite/23_containers/multimap/allocator/move_cons.cc: 
>New test.
>            * testsuite/23_containers/multiset/allocator/move_cons.cc: 
>New test.
>            * testsuite/23_containers/set/allocator/move_cons.cc: New test.
>
>Ok to commit once all tests have complete ?

Yes, OK for trunk, thanks!