Re: [Boost-bugs] [Boost C++ Libraries] #7718: Basic rvalue and C++11 support (part 2)

Subject: Re: [Boost-bugs] [Boost C++ Libraries] #7718: Basic rvalue and C++11 support (part 2)
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2013-01-06 01:39:34

#7718: Basic rvalue and C++11 support (part 2)
-------------------------------+--------------------------------------------
  Reporter:  apolukhin         |       Owner:  ebf         
      Type:  Patches           |      Status:  reopened    
 Milestone:  To Be Determined  |   Component:  variant     
   Version:  Boost 1.52.0      |    Severity:  Optimization
Resolution:                    |    Keywords:              
-------------------------------+--------------------------------------------
Changes (by djowel):

  * status:  closed => reopened
  * resolution:  fixed =>

Comment:

 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.

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/7718#comment:6>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.