[boost] [optional] problems with swap()

From: Thorsten Ottosen (thorsten.ottosen_at_[hidden])
Date: 2008-04-10 07:08:11

Hi Fernando,

I have some comments on the way optional<T>::swap() is implemented. I 
looks like this:

// optional's swap:
// If both are initialized, calls swap(T&, T&). If this swap throws, 
//both will remain initialized but their values are now unspecified.
// If only one is initialized, calls U.reset(*I), THEN I.reset().
// If U.reset(*I) throws, both are left UNCHANGED (U is kept 
//uinitialized and I is never reset)
// If both are uninitialized, do nothing (no-throw)
template<class T>
inline
void optional_swap ( optional<T>& x, optional<T>& y )
{
   if ( !x && !!y )
   {
     x.reset(*y);
     y.reset();
   }
   else if ( !!x && !y )
   {
     y.reset(*x);
     x.reset();
   }
   else if ( !!x && !!y )
   {
// GCC > 3.2 and all other compilers have the using declaration at 
function scope (FLC)
#ifndef BOOST_OPTIONAL_STD_SWAP_INTRODUCED_AT_NS_SCOPE
     // allow for Koenig lookup
     using std::swap ;
#endif
     swap(*x,*y);
   }
}

The problem here is that the two first cases do not delegate to the 
version of swap found by ADL. That only happens in cases where both 
objects are initialized. This is, AFAICT, a major problem because 
copy-construction has quite different semantics than swap() when it 
comes to invalidation of iterators or pointers. It works ok for ints, 
but creates havoc when using a container with internal memory.

I suggest we do something along these lines:

template<class T>
inline void optional_swap ( optional<T>& x, optional<T>& y )
{
     bool hasX = x;
     bool hasY = y;

     if ( !hasX && !hasY )
       return;

     if( !hasX )
         x = boost::in_place();
     else if ( !hasY )
         y = boost::in_place();

   // GCC > 3.2 and all other compilers have the using declaration at 
function scope (FLC)
#ifndef BOOST_OPTIONAL_STD_SWAP_INTRODUCED_AT_NS_SCOPE
     // allow for Koenig lookup
     using std::swap ;
#endif
     swap(*x,*y);

     if( !hasX )
         y.reset();
     else if( !hasY )
         x.reset();
}

Comments?

-Thorsten