Joel de Guzman wrote:
Hi,
I just looked at the current state of variant and noticed that the implementation of recursive_variant's move ctor seems to be not as optimal as I hoped. The current implementation as written is:
recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand) : p_(new T( detail::variant::move(operand.get()) )) { }
Unless I am mistaken, I think the heap allocation is not needed and the target should simply grab the pointer and mark the source pointer as 0 (with additional checks for 0 in the dtor):
template <typename T> recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand) : p_(operand.release()) { }
template <typename T> T* recursive_wrapper<T>::release() { T* ptr = p_; p_ = 0; return ptr; }
template <typename T> recursive_wrapper<T>::~recursive_wrapper() { if (p_) boost::checked_delete(p_); }
The tests are running just fine (tested with gcc, msvc) with this (more optimal) implementation. I also run the Spirit test suite (Spirit makes heavy use of variant).
Thoughts? I can commit this patch if it is acceptable.
A recursive_wrapper is not a pointer. It's a value-like wrapper that is assumed to always contain a valid object. The move constructor should leave the moved-from recursive_wrapper in a valid state, which precludes nullifying it. That is, unless you suggest adding an "empty" state to recursive_wrapper, which doesn't sound like a very good idea. -- Paul Smith -- View this message in context: http://boost.2283326.n4.nabble.com/variant-Basic-rvalue-and-C-11-features-su... Sent from the Boost - Dev mailing list archive at Nabble.com.