Forward list default default and move constructors

[François Dumont]

On 05/07/2017 18:10, Jonathan Wakely wrote:
> On 19/06/17 22:48 +0200, François Dumont wrote:
>> Hi
>>
>>    Here is the patch to default the default and move constructors on 
>> the std::forward_list. Putting a move constructor on 
>> _Fwd_list_node_base helped limiting the code impact of this patch. It 
>> doesn't have any side effect as iterator types using this base type 
>> are not defining any move semantic.
>
> I don't understand this comment.
>
> 1) The iterators only _Fwd_list_node_base* pointers, so that's why
> they aren't affected. It's not because the iterators don't define move
> semantics.
>
> 2) The iterators *do* have move semantics, they have
> implicitly-declared move operations, which are identical to their
> implicitly-defined copy operations (because moving a pointer is
> identical to copying it).
>
> 3) Adding this move constructor has a pretty large side effect because
> now its copy constructor and copy assignment operator are defined as
> deleted, and it has no move assignment operator. That's OK, because we
> never copy or move nodes (except in the new _Fwd_list_impl move ctor
> you're adding). But it's a significant side effect. Please consider
> adding the following to make those side effects explicit:
>
>  _Fwd_list_node_base(const _Fwd_list_node_base&) = delete;
>  _Fwd_list_node_base& operator=(const _Fwd_list_node_base&) = delete;
>  _Fwd_list_node_base& operator=(_Fwd_list_node_base&&) = delete;

Yes, sorry, my comment was indeed wrong. I should have limit it to say 
that I was not seeing real side effect considering current code. I added 
those explicit special deleted members.

>
>
>>    I also took the time to optimize the move constructor with 
>> allocator when allocator is always equal. It avoids initializing an 
>> empty forward list for nothing.
>>
>>    I think it is fine but could we have an abi issue because of the 
>> change in forward_list.tcc ?
>
> Old code with undefined references to that constructor will still find
> a definition in new code that explicitly instantiates a forward_list.
>
> New code compiled after your change would not find the new
> constructors (the ones with true_type and false_type parameters) in
> old code that explicitly instantiated a forward_list.
>
> Could you split that part of the change into a separate patch? The
> changes to define constructors as defaulted are OK, so I'd like to
> considere the proposed optimisation separately 

Done in attached patch, ok to commit ?

François

-------------- next part --------------
A non-text attachment was scrubbed...
Name: forward_list.h.patch
Type: text/x-patch
Size: 5610 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/libstdc++/attachments/20170713/65f09cc1/attachment.bin>