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: Peter Dimov (pdimov_at_[hidden])
Date: 2009-09-07 16:44:24

Howard Hinnant wrote:
> On Sep 7, 2009, at 3:26 PM, Christopher Jefferson wrote:
>
>>
>> On 7 Sep 2009, at 20:08, Jeffrey Hellrung wrote:
>>
>>> Howard Hinnant wrote:
>>>> Using an experimental C++0x compiler and library (you might can do
>>>> this experiment g++-4.4, I'm not sure) I make this substitution
>>>> and indeed, I get the same output (remove all of the *.dat files
>>>> prior to each run).  This experimental library has:
>>>> template <class _Tp, class _Allocator>
>>>> inline
>>>> vector<_Tp, _Allocator>&
>>>> vector<_Tp, _Allocator>::operator=(vector&& __x)
>>>> {
>>>>   clear();
>>>>   swap(__x);
>>>>   return *this;
>>>> }
>>>
>>> Perhaps a dumb question, but isn't the above incorrect under self-
>>> assigment (x = move(x))?  Is there a "standard way" or "recommended
>>> guideline" for handling self-assignment in the move assignment
>>> operator, or is this a nonissue?

It's incorrect under self-assignment. The correct version is

T& operator=( T&& x )
{
    T( move(x) ).swap( *this );
    return *this;
}

or its inlined equivalent.

>> I'm actually not 100% positive what the intended semantics of x =
>> move(x) are, but clearly as a QOI issue it should behave.
>>
>> Most people put:
>>
>> if(&__x != this)
>>
>> or something similar in normal assignment operators, adding it also
>> to all move operators seems very sensible.

This is not enough in general. clear() may end up indirectly destroying the 
contents of x, even when &x != this. (Although... now that I think of it... 
this may be a problem with the vector copy assignment as well.)

> My hope has been to make x = move(x) undefined behavior.

I find this a bit hard to justify.

> Rationale:
> The move assignment operator should be fast enough that an extra if-
> test may adversely impact performance (above the noise level).

The problem with making it UB is that y = move(x) now needs to be guarded 
with if( &x != &y ) everywhere in client code. Sometimes the user will know 
that x is not y, but is this really the case most of the time?