Boost logo

Boost :

From: Andrey Semashev (andysem_at_[hidden])
Date: 2008-04-10 13:01:37

Thorsten Ottosen wrote:
> 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)


> 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:


> Comments?

Your approach requires the T to be default constructible which is not
always the case. Besides, to my mind, the current implementation is more
obvious for the user.

Your behavior can be implemented in a non-intrusive way something like that:

   template< typename T >
   void force_koenig_swap(optional< T >& left, optional< T >& right)
     bool left_empty = !left;
     if (left_empty) left = in_place();
     bool right_empty = !right;
     if (right_empty) right = in_place();

     using std::swap;
     swap(left.get(), right.get());

     if (left_empty) right = none;
     if (right_empty) left = none;

As for optional, I think, move construction would be more appropriate in
case if one of the optionals is empty. And if T does not support moving
the current implementation is good enough.

Boost list run by bdawes at, gregod at, cpdaniel at, john at