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: Christopher Jefferson (chris_at_[hidden])
Date: 2009-09-07 17:20:20

On 7 Sep 2009, at 21:44, Peter Dimov wrote:
>
>
>>> 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?

Actually thinking about it, I wonder if when this occurs it is a  
mistake -- should instead a debug mode assert if &x == &y?

x = move(x) actually seems like it would be a logic error. Certainly  
having now implemented a reasonable amount of rvalue code, including a  
standard library implementation, I can't think of where it would arise  
without an error. move(x) implies we do not care about the value of x,  
but then the assignment means that we do care.

Certainly I'd want to see an assert in debug mode, to catch this, as I  
imagine errors it causes could be quite subtle.

Chris