Re: [boost] [move][container] Review Request (new versions ofBoost.Move and Boost.Container in sandbox and vault)

Subject: Re: [boost] [move][container] Review Request (new versions ofBoost.Move and Boost.Container in sandbox and vault)
From: Howard Hinnant (howard.hinnant_at_[hidden])
Date: 2009-09-07 21:07:47

On Sep 7, 2009, at 8:37 PM, David Abrahams wrote:

>
> on Mon Sep 07 2009, Howard Hinnant <howard.hinnant-AT-gmail.com>  
> wrote:
>
>> std::swap will only do a self-move-assignment using a moved-from
>> object.
>
> You mean the target will be a moved-from object.

<nod>

>  OK, so?

So the target is resource-less.  There are no pre-existing resources  
the target must get rid of prior to the move-assignment.  That means  
you're not going to delete yourself, even if the move assignment is  
self-referencing.

>> Of course
>> vector will never use this definition.  But if it  did, then:
>>
>> template <class _Tp, class _Allocator>
>> inline
>> vector<_Tp, _Allocator>&
>> vector<_Tp, _Allocator>::operator=(vector&& __x)
>> {
>>   clear();
>>   swap(__x);
>>   return *this;
>> }
>>
>> would work just fine.  clear() is a no-op, and then you swap an  
>> empty capacity vector
>> with itself.
>>
>> My point is that I don't want to be told I need to put a if(this !=
>> &x) in my move assignment operator.  It is a useless performance
>> penalty.  Everyone should not have to pay (in terms of lost
>> performance) for someone's bad code just to make sure self-move-
>> assignment does a no-op instead of crashes.
>
> And the reason this argument doesn't also apply to copy assignment
> is...?

swap doesn't use copy assignment. ;-)

More seriously because:

> I'm currently thinking that we need a Chapter 17 statement along the  
> lines of:
>
>   The C++ standard library may assume that an rvalue reference  
> represents a unique reference to the bound object.

This simple statement removes sometimes-expensive error checking that  
is vanishingly-rarely useful.  One can not self-assign a true (non- 
casted) rvalue.  Move semantics only works because the code with the  
reference to the rvalue can assume that it has the *only* reference to  
that rvalue. (this is sort of like C99's restrict in reverse)

>> If you want to cast an lvalue to an rvalue, that's fine.  Just make
>> sure your code logic is such that such a cast is safe.  I believe
>> std::swap's use of move assignment is safe because by the time it
>> move-assigns, if it is self referencing, all of your resources have
>> already been moved to a local temp.
>
> Then what is the coding guideline, exactly?  "Make sure it's safe"
> doesn't qualify.

Agreed.  A good guideline might go along the lines of:  if you move  
assign, the target should generally be moved from first.  If it is  
not, ensure that target and source do not refer to the same object.

This is not as hard as it sounds:  Every single in-place-sequence- 
permuting algorithm in <algorithm>, including swap, sort, rotate,  
random_shuffle, partition, nth_element, etc. works by either swapping,  
or by move-constructing an element out to a local temp, and then move  
assigning into the "hole" (and then move assigning into the new  
"hole").  I.e. these algorithms always move assign into a moved-from  
element.  This behavior also includes the container member algorithms  
for vector/deque insert.

An exception to the above statement is those algorithms which  
"shorten" the sequence: remove and remove_if, and the container member  
functions erase (for vector and deque).  Here the initial move  
assignment is into a desired-removed but not moved-from element.  This  
algorithm will typically result in the trailing "removed" elements all  
being moved-from.  In these algorithms, there is no chance of move  
assignment being self referencing (except for a coding error on the  
part of the author of remove/erase, etc.).

-Howard