[boost] Re: [bug]: optional<T> fails on self-assignment

From: Fernando Cacciola (fernando_cacciola_at_[hidden])
Date: 2005-02-15 12:54:19

"Joe Gottman" <jgottman_at_[hidden]> escribió en el mensaje 
news:curjg1$2vq$1_at_sea.gmane.org...

>   I just discovered that optional<T> is not safe under self-assignment.

Ouch!! This is a major oversight from my part...
Every C++ 101 book warns about it

> Consider the following code fragment:
>
>    boost::optional<long> x(1); //At this point x is initialized and *x == 
> 1
>    x = x; //x should be unchanged, but instead x is now uninitialized
>
> Optional's assignment operator looks like
>
> optional& operator= ( optional const& rhs )
>      {
>        this->assign( rhs ) ;
>        return *this ;
>      }
>
> and assign is defined by
>
> void assign ( argument_type val )
>      {
>        destroy();
>        construct(val);
>      }
>

Well, this is not the correct overload, but you are right all the same.

> Thus, if (this == &rhs) in operator=(), any value held by rhs will be 
> destroyed.

Indeed (what was I thinking!)

>
> I can see two possible fixes for this.  the first is to put an "if (this 
> != &rhs) " around the call to assign in operator=().  This is simplest, 
> but it slows the code down in order to protect against a very rare 
> situation.
> The other is to rewrite assign as follows:
>
> void assign(argument_type val)
> {
>    if (m_initialized) {
>        get()  = val; // call assignment operator
>    } else {
>        construct(val); //call copy constructor
>    }
>    return *this;
> }
>
> This makes optional self-assignment safe.

Right.

> It has the added advantage that if T's assignment operator has the strong 
> exception safety guarantee, so does optional<T>'s assignment operator.

Well, this is a good point.
It is typical for classes with a throwing copy ctor to have a strong 
assignment.

> It has the disadvantage of making optional depend on both assignment 
> operator and copy constructor to copy, but very few classes have a copy 
> constructor but don't have an assignment operator.
>
Right, I don't think this extra requirement will have any problems in 
practice.

Thanks for the report!

Fernando Cacciola