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)
[snip]
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:
[snip]
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.