Re: forward list patch

[François Dumont <francois dot cppdevs at free dot fr>]

On 10/23/2010 12:20 AM, Paolo Carlini wrote:
> On 10/22/2010 11:19 PM, François Dumont wrote:
>    
> > Hi
> >
> >      While working on debug mode contention issue I discovered some
> > bugs that I prefer to submit first. Here is a first patch for the
> > forward_list. It fixes the move constructor that was missing the move
> > of safe iterators and introduce a special _M_swap implementation that
> > do not swap the before begin iterator that cannot move from a
> > forward_list instance to an other.
> >      
> Gosh, the new _M_swap_aux is *huge*, certainly we want it out of line.
> Anyway, could you please provide a bit of rationale about it? I have
> some trouble believing we really need something such complex, so special
> to forward_list. And now I see even __scoped_lock in<forward_list>
> itself, again something that does not happen for all the other
> containers. I think we should give these issues more thought.
>
> Thanks,
> Paolo.
>    
Yes, I agree that the new _M_swap is big and is not something you would 
expect in a swap implementation. The problem rely in the before_begin 
and swap  implementations:

      const_iterator
      before_begin() const
      { return const_iterator(&this->_M_impl._M_head); }

      void
      swap(forward_list& __list)
      { std::swap(this->_M_impl._M_head._M_next, 
__list._M_impl._M_head._M_next); }

You see, the swap operation swap only starting at begin iterator, but 
before begin never gets swap. The added test swap.cc show this problem. 
When run in normal mode it works, when run in debug mode it fails 
without the patch because the debug mode signal that the test is trying 
to compare iterators from different containers.

Are the specifications saying something about management of before_begin 
on swap operation ? If it says nothing we could perhaps change the 
forward_list implementation so that even the before begin iterator comes 
from a dynamic allocation that could then be swap like the others. 
However it would be abnormal to change the normal implementation because 
of an issue in the debug mode.

Do you want me to move it to a forward_list.tcc file ?

Regards