Re: iter_swap on move_iterator

[Christopher Jefferson <caj21 at st-andrews dot ac dot uk>]

On 19/05/12 01:07, Jonathan Wakely wrote:
> On 18 May 2012 19:12, Daniel Krügler wrote:
> > 2012/5/18 François Dumont<frs.dumont@gmail.com>:
> > > On 05/17/2012 08:47 PM, Daniel Krügler wrote:
> > > > 2012/5/17 François Dumont<frs.dumont@gmail.com>:
> > > > > Hi
> > > > >
> > > > >     While working on removal of code duplications in algos implementation
> > > > > I
> > > > > challenge them with move_iterator. I started with stable_sort and it
> > > > > doesn't
> > > > > compile. I summarize the problem to the fact that iter_swap can't be used
> > > > > on
> > > > > move_iterator. As, in C++11, iter_swap simply forward to swap(*__a, *__b)
> > > > > it
> > > > > is normal that it can't accept rvalue but shouldn't iter_swap be adapted
> > > > > to
> > > > > accept move_iterator ?
> > > > Yes, when the referenced type (or the iterator's reference) provides a
> > > > swap overload
> > > > that accepts rvalues. Remember that move_iterator will produce an rvalue
> > > > on dereference, this rvalue is provided to the swap overload that is found
> > > > by ADL. Given your description I would expect that the test type does not
> > > > provide a swap overload accepting rvalues, thus the std::swap is selected,
> > > > but cannot accept the arguments.
> > > Ok for how to have iter_swap work with move_iterator, users must grant the
> > > necessary swap overload. But what about the other algos like stable_sort I
> > > have tested at first ? Shouldn't they be adapted to work with move_iterator
> > > ?
> > I don't think so. The library requirements on the iterator type is
> > ValueSwappable, which
> > means that "for any dereferenceable object x of type X, *x is
> > swappable". So, unless
> > there exists active rvalue-swap support provided by user-provided value type of
> > the wrapped iterator of std::move_iterator (or by the reference type
> > of the wrapped
> > iterator, once LWG 2106 becomes accepted), the library implementation should
> > not interfere here.
> I agree, and will just add that support for swapping rvalues of
> standard library classes was added to a C++0x draft but then
> intentionally removed again as undesirable and IIRC in some cases
> unsafe.

Furthermore, we used to use move_iterator internally in std::sort, and 
had to take it out because it would sometimes lead to values being lost, 
if the type had a 'by value' operator< like:

bool operator<(MyType t1, MyType t2) { ... }

Because as soon as we compared we would move the value into the 
comparitor and it would be lost.

I would personally advise against supporting move_iterator in the 
standard algorithms, unless you are also prepared to make it safe in 
such situations (which might itself be impossible, without breaking 
other things).

In short, move_iterator is in my opinion easily misused and not a proper 
iterator and not compiling it might be a 'feature' :)

Chris