[Boost-users] [optional] new assignmet semantic and references

From: Fernando Cacciola (fernando_cacciola_at_[hidden])
Date: 2005-03-01 11:51:45

{This is copy of a message on the developers list that needs attention from 
Boost.Optional Users}

Hello people,

A few days ago, a problem with Boot.Optional's assignment not properly
handling aliasing scenarios (like self assignment) was pointed out. We
concluded that the solution was to use T's operator=() directly instead of
the Destroy+Construct pattern that I was using.
I changed the code, tested it, and I was updating the documentation when I
recalled the following long-standing issue:
Currently, with the Destroy+Construct pattern, references are re-bound in
case of assignment:

int a = 123 ;
int b = 456 ;
int& ra = a ;
int& rb = rb ;
optional<int&> ora(ra);
optional<int&> orb(rb);
ora = orb ; // now 'ora' contains a reference to 'b' instead of 'a'
b = 789 ;
cout << *ora ;  Prints 789
cout << a ; Prints 123

This is not the way assignment of reference types work in C++:

ra = rb ; // The value of 'a' referenced by 'ra' is changed, not the
reference itself.
cout << a ; Prints 789

This error in the way optional<T&> treated assignment of reference types was
pointed out by Joel de Guzman long ago when he started using Optional in
Spirit. However, at the time, Joel explicitely asked me not to change this
and keep the current rebinding semantics in order to minimize any impact in
Spitit itself (which certainly has, o had, a much larger user base).

If I change the code now to use T's operator=(), assignment of optional<T&>
will have a _radically_ different semantics. A correct one, that's for sure,
but end user code might be fundamentally impacted.

Note: if you look at Optional documentation you will discover that
assignment is only documented for T not being of reference type. This was
intentional.. I left this undocumented becasue I knew back then that the
current semantics were wrong but I couldn't fix it at the time because
Spirit was (or could be) depending on it; yet I didn't want other users to
depend on it as well... Of course, users could have "discovered" the
(incorrect) semantics themselves and now rely on it, but I hope that the
intentionally missing documentation served to keep the amount of users
depending on this incorrect semantics very low. [At least it served _me_ to
remember about this since I had forgotten]

I really want to 'fix' this, i.e, give it correct semantics, so unless Joel
or anyone else speaks now I'll move on, finish the documentation updates and
commit the new version.

Best,

Fernando Cacciola

P.S: Joel already agreed in the developers list that this needs to be fixed.