This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Forward list default default and move constructors


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;


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.





Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]