Re: [boost] [variant] Basic rvalue and C++11 features support

Subject: Re: [boost] [variant] Basic rvalue and C++11 features support
From: Joel de Guzman (djowel_at_[hidden])
Date: 2013-01-05 20:40:20

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.

Regards,
-- 
Joel de Guzman
http://www.boostpro.com
http://boost-spirit.com