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-25 07:00:26

Ion Gaztañaga wrote:
> Thomas Klimpel wrote:
> > 4) The old library implements adobe::move() and automatically move
> > assigns from temporaries.
> [snip]
> > It seems to me
> > that the existing implementation provides (4), but the existing
> > documentation doesn't clearly state this.
> 
> I'm sorry, I don't fully understand this. What do you mean by
> "automatically move assigns from temporaries"? That the existing
> implementation allows catching rvalues if the assignment operator
> catches by value?

Yes. But from my point of view, this is something non-trivial, and it works for both BOOST_MOVABLE_BUT_NOT_COPYABLE and BOOST_COPYABLE_AND_MOVABLE classes. For BOOST_COPYABLE_AND_MOVABLE, you have to implement a move copy constructor and implement the assignment operator taking its arguments by value. For BOOST_MOVABLE_BUT_NOT_COPYABLE, you have to implement a move assignment operator and declare the non-const reference assignment operator as private. 

Even if it may have been obvious to you, I had to think about how the different constructors and assignment operators would kick in to see that it will really work.

> > With respect to the macros, I think one could try to separate the
> > "usability" macros (like BOOST_MOVABLE_BUT_NOT_COPYABLE and
> > BOOST_COPYABLE_AND_MOVABLE) from the macros that are required to
> > achieve (2).
> 
> What do you mean with "separate"? BOOST_MOVABLE_BUT_NOT_COPYABLE &
> BOOST_COPYABLE_AND_MOVABLE ARE "usability" and the rest are
> required for (2)?

But you currently can't get (2) without using some of the "usability" macros. 

> > It's not bad, but it has quite some room for improvement. And
> > "two_emulation_modes.html" is simply not acceptable in its current
> > form.
> 
> Ok. I would need a bit of input on how to explain different options
> offered by the library.

OK, I will try later after the technical questions are settled.

> Ok, but we need a tutorial for newcomers that don't care about language
> issues, just how to make their class movable.

But this tutorial is already part of the current documentation, or do I misunderstand something?

> Both approaches can coexist. We just need to offer another macro for
> BOOST_OPTIMIZED_COPYABLE_AND_MOVABLE. If choice is what boosters
> prefers, I'm open to that, my main goal was to push a move emulation
> that has been blocked for months, if not for years.

I don't think that we will lose much by offering both modes, so it would be fine with me. 

> Regarding containers, I think (please, correctme if I'm wrong) we can
> only move_assign rvalues in non-optimized mode catching by value. 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.

> The problem is what should generic algorithms
> should expect from a class, because for an object in non-optimized
> mode,
> assignment from a rvalue might not by the best way to implement an
> algorithm, but the algorithm does not know that (and even this has
> consequences with exception guarantees, but this is another issue).

Yes, this is an interesting question. In my opinion, breaking move assignment from temporaries is worse than not reusing existing resources, so for me the non-optimized mode is the one that catches by value for BOOST_COPYABLE_AND_MOVABLE classes.

Regards,
Thomas