Re: [boost] [Review] Formal Review: Boost.Move

Subject: Re: [boost] [Review] Formal Review: Boost.Move
From: Thomas Klimpel (Thomas.Klimpel_at_[hidden])
Date: 2010-05-30 20:21:56

Ion Gaztañaga wrote:
> >> And this is a big pessimization for container copy
> >> constructors because the container does not reuse existing
> >> resources:
> >
> > Yes, but the reuse of existing resources is not without drawbacks
> > (independent of the problem with the non-const assignment operator).
> > So I wouldn't object to have the option to use the optimized mode with
> > containers, but the non-optimized mode is more important, so if there
> > has to be a choice, the non-optimized mode should win.
> 
> Well, I gently disagree, but maybe you are right and others also prefer
> the non-optimized mode. We'll come back to this issue if someday we
> review Boost.Container ;-)

Perhaps you are right. I recently realized that you worry about the copy assignment operator not reusing existing resources, not about the move assignment operator (you wrote copy constructor and I just read this as assignment operator). It's true that the copy assignment operator of std::vector is normally implemented so that it reuses existing resources. So preserving this behavior is not so much a question about whether this behavior is a good idea, but a question about backward compatibility.

The main drawback of the optimized move emulation mode is that it forces other classes to also implement an optimized move emulation, if they want to derive from an optimized class or use it as member. I have the impression that this problem can be solved by introducing a suitable macro (let's call it BOOST_MOVE_ASSIGNMENT_WRAPPER), that could be defined as follows:

#if defined(BOOST_NO_RVALUE_REFERENCES) && !defined(BOOST_MOVE_DOXYGEN_INVOKED)

namespace boost {
template <class OpimizedCopyAndMove>
class move_assignment_wrapper : public OpimizedCopyAndMove
{
  typedef move_assignment_wrapper<OpimizedCopyAndMove> self_t;
  typedef OpimizedCopyAndMove base_t;
public:
  assignment_wrapper(const self_t& s) : base_t(s) {}
  assignment_wrapper(BOOST_RV_REF(self_t) s) : base_t(boost::move(static_cast<base_t&>(s))) {}
  self_t& operator=(self_t s) { base_t::operator=(boost::move(static_cast<base_t&>(s))); return *this; }

  assignment_wrapper(base_t& b) : base_t(b) {}
  assignment_wrapper(BOOST_RV_REF(base_t) b) : base_t(b) {}
  assignment_wrapper(const BOOST_RV_REF(base_t) b) : base_t(b) {}
  self_t& operator=(base_t& b) { base_t::operator=(b); return *this; }
  self_t& operator=(BOOST_RV_REF(base_t) b) { base_t::operator=(b); return *this; }
  self_t& operator=(const BOOST_RV_REF(base_t) b) { base_t::operator=(b); return *this; }
};
}  // namespace boost

#define BOOST_MOVE_ASSIGNMENT_WRAPPER(TYPE)\
  boost::move_assignment_wrapper<TYPE>\
//

#else  //BOOST_NO_RVALUE_REFERENCES

#define BOOST_MOVE_ASSIGNMENT_WRAPPER(TYPE)

#endif

So it is now possible to derive from an optimized class or use it as member without being forced to also implement an optimized move emulation. All that is needed to to wrap its type declaration with the BOOST_MOVE_ASSIGNMENT_WRAPPER macro. As implemented above, constructor forwarding is  still missing,  but as boost.move also offers constructor forwarding functionality, it should be possible to fix this. In addition, there might be ADL issues, but even these can probably be fixed.

However, my position on breaking move-assignment from temporaries for the non-optimized mode remains that this is not a reasonable option, especially considering that it would be a serious regression with respect to the Adobe Move Library.

Regards,
Thomas