iter_swap on move_iterator

[Christopher Jefferson]

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