on Wed Aug 12 2009, Ion Gaztañaga <igaztanaga-AT-gmail.com> wrote:

David Abrahams escribió:

...

Very great news to hear that this project is moving forward (so to speak ;-))

Let's hope this is the final step!

...

...

The library has now new macros to declare copyable and movable types and movable only types. Old macros are still there for backwards compatibility (specially unordered library which was also ported to this move sandbox) but I expect to remove them if current approach is considered better.

->Updated documentation and tests.

http://svn.boost.org/svn/boost/sandbox/move/libs/move/doc/html/move/composit...

...

still says "all moves occur explicitly," but I think you mean "all moves from lvalues," right?

Yes. The new library now moves non-const rvalues in assignments:

copyable_and_movable produce();

copyable_and_movable cm; for(;;){ //now moved instead of copied! cm = produce(); }

Only in assignments? what about consume(produce()) scenarios?

This is not the case with constructors (the original Klaus Triendl proposal included them, http://lists.boost.org/Archives/boost/2009/06/153266.php) because it terribly uglifies syntax

Are you sure you've looked at all the alternatives? Every move library I'm aware of before yours supports implicitly move constructing from non-const rvalues.

(there is no way in C++03 to automatically generate constructor overloads that forward to another constructor) and because that case is in practice more efficient than moving thanks to RVO:

for(;;){ //Copied? No, RVO avoids copy copyable_and_movable cm = produce(); }

You might check whether direct initialization is just as efficient as copy initialization before you say that: copyable_and_movable cm((produce())); IIRC EDG-based compilers used to only elide copies in the copy initialization case (!) -- Dave Abrahams BoostPro Computing http://www.boostpro.com

Looks good; personally I've been waiting for this so I'm extremely pleased to see the great work Ion has done here! A few small suggestions: Is BOOST_MOVABLE_BUT_NOT_COPYABLE better called BOOST_MOVABLE_AND_NOT_COPYABLE? Or even BOOST_MOVABLE_NOT_COPYABLE or BOOST_MOVABLE_NO_COPY Specifically, why the exceptional 'but' rather than an inclusive 'and'; or just elide the conjunction altogether? Should the limit BOOST_CONTAINERS_MAX_CONSTRUCTOR_PARAMETERS use BOOST_CONTAINER_... (ie, drop the pluralising 'S')? Similarly for other pre-processor symbols; some use BOOST_CONTAINERS_ and others use BOOST_CONTAINER_ I note that there is a containers_detail namespace; since this this not part of the public interface there is no case for renaming this to container_detail other than internal consistency. The stable_vector is nice (same as chain<T>), just some of the pre-processor symbols don't have BOOST_ prefixes, and CONTAINERS_ vs. CONTAINER_ comes up again. This is niggly, sure. I'm pleased that Ion has taken the time to do this, and do it well. Now I can finish monotonic with both stateful and stateless allocators. Christian

Ok, but I would like to avoid any header dependency with MPL so that library is minimal. Just jumping in on this. To have your meta-function MPL compliant, I

Ion Gaztañaga wrote: think you may just need them to export their result type as a typedef named type or as a internal template class named apply. This is ofc doesn't require any mpl header. -- ___________________________________________ Joel Falcou - Assistant Professor PARALL Team - LRI - Universite Paris Sud XI Tel : (+33)1 69 15 66 35

Ion Gaztañaga wrote:

Thanks for the comments!

No problem.

...

- Make all metafunctions true MPL metafunctions (as mentioned in previous postings).

Ok, but I would like to avoid any header dependency with MPL so that library is minimal.

That's fair, though as noted by others, all you need to do us add a nested type typedef which has a nested value constant, which (I think) can even be the metafunction class itself. You'd still keep the value bool constant in the original class as well. Example (not 100% sure this is correct): template< class T > struct metaf { // Or use BOOST_STATIC_CONSTANT( ... ) ? static const bool value = ...; typedef metaf type; }; - Jeff

Jeffrey Hellrung escribió:

Ion,

With your current move-emulation approach, it looks like functions like vector::push_back could actually capture genuine rvalues, similar to how operator= does, via BOOST_COPY_ASSIGN_REF( T ) for movable types T (and using const T& for nonmovable types). For example:

Yes, I know that, but I had no time to update containers to take advantage of this feature. I'll try to do it before the review, I think you need 3 overloads for C++03 systems: //const T & for C++0x, const boost::rv<T> & for C++03 push_back(BOOST_COPY_ASSIGN_REF(value_type)v); //T && for C++0x, boost::rv<T> & for C++03 push_back(BOOST_RV_REF( T ) value); #ifdef BOOST_HAS_RVALUE_REFS push_back(value_type & v) { push_back(const_cast<const value_type &>(v); } #endif However, I haven't found a way to generalize it for perfect forwarding (emplace, emplace_back) because the template parameters must be deduced from arguments and that does not work as push_back. I need help from more talented people ;-) Best, Ion

Jeffrey Hellrung escribió:

Great, just making sure that was on the table ;) Also, surely you mean

#ifndef BOOST_HAS_RVALUE_REFS // rather than #ifdef

Yes ;-)

As far as generic forwarding, I've been playing around with it a little and I'm likewise stuck on how to capture true rvalues via template parameter deduction :/

It's not trivial... And if we more than 2 overloads for each parameter, we'll need to produce a ton of preprocessed code ;-) Best, Ion

Jeffrey Hellrung wrote:

I've used Boost.Preprocessor to generate 2^n overloads of T&/const T&

There should be no need to write such a thing yourself, as Boost.Functional/Forward already does it.

Mathias Gaunard wrote:

Jeffrey Hellrung wrote:

...

I've used Boost.Preprocessor to generate 2^n overloads of T&/const T&

There should be no need to write such a thing yourself, as Boost.Functional/Forward already does it.

I can't find this component in the boost documentation section, a quick google search, sandbox svn repo, or vault, and searching for "forward" in the boost source doesn't look like it gives anything useful. Help? OvermindDL1 wrote:

2009/8/28 Ion Gaztañaga <igaztanaga@gmail.com>:

...

It's not trivial... And if we more than 2 overloads for each parameter, we'll need to produce a ton of preprocessed code ;-)

I thought Boost.Fusion could ease most of that without preprocessed code?

If you're willing to force the client to wrap arguments up into a fusion vector, correct? I only took a superficial look at the fusion/functional section of the fusion docs, assuming that's what you're referring to. - Jeff

2009/8/28 Ion Gaztañaga <igaztanaga@gmail.com>:

Jeffrey Hellrung escribió:

...

Great, just making sure that was on the table ;)  Also, surely you mean

#ifndef BOOST_HAS_RVALUE_REFS // rather than #ifdef

Yes ;-)

...

As far as generic forwarding, I've been playing around with it a little and I'm likewise stuck on how to capture true rvalues via template parameter deduction :/

It's not trivial... And if we more than 2 overloads for each parameter, we'll need to produce a ton of preprocessed code ;-)

I thought Boost.Fusion could ease most of that without preprocessed code?

Thomas Klimpel wrote:

I would say that if an automatic "bad smell" detection tool like Coverity would flag the above code as dangerous, it wouldn't be completely wrong. After mx was used via boost::move(mx), it is not supposed to be used again. So I think it would be better to write

vector(BOOST_RV_REF(vector) mx) : base_t(mx) { this->swap(mx); }

which is also less confusing for a human reader.

If base(const base_t & rhs) copies rhs into this and if swap(...) swaps everything (including base_t part of a verctor) then the base_t part of mx will be first copied into this and then swaped back into mx. I do not think this can cause errors unless there is something special about slicing a vector instance, but the copy seems to be redundant.

Thomas Klimpel escribió:

I would say that if an automatic "bad smell" detection tool like Coverity would flag the above code as dangerous, it wouldn't be completely wrong. After mx was used via boost::move(mx), it is not supposed to be used again. So I think it would be better to write

vector(BOOST_RV_REF(vector) mx) : base_t(mx) { this->swap(mx); }

which is also less confusing for a human reader.

Yes, but that wouldn't be the right thing. When writing move constructors one must first move base classes and then members.Otherwise, the base it's copied instead of moved. Anyway, I think this should be reflected better casting mx to base_t and then moved.

I would argue that the implementation is not consistent with the documentation. The x.clear() statement can cause up to n destructors to be called, violating the "<b>Complexity</b>: Constant." claim.

You are right.

The x.clear() statement also has a bad interaction with the "has_trivial_destructor_after_move" template specialization at the end of the vector.hpp file:

You are right. clear() was added later in the design so there are some flaws regarding these issues. Thanks for spotting this.

So a fixed version could look like

//! <b>Postcondition</b>: x.empty() && !x.capacity(). *this contains a the elements x had //! before the function. ... //! <b>Complexity</b>: Linear to the number of elements in the vector (destructor calls). vector& operator=(BOOST_RV_REF(vector) x) { if (&x != this){ this->swap(x); x.clear(); shrink_to_fit(); } return *this; }

But one can wonder whether it would not be better to completely omit the x.clear() statement. After all, x is ("equivalent to") an r-value, so it can be treated as if it will be destroyed immediately after the function call. It's the responsibility of the user that tags an function argument as r-value by using boost::move to ensure that it can be treated just like an r-value.

This can lead to suprissing behaviours in the presence of some reference counted values: vector<shared_ptr<T>> v1 (...); shared_ptr<T> t1(new T(...)); v1.push_back(move(t1)); vector<shared_ptr<T>> v2 (...); shared_ptr<T> t2(new T(...)); v2.push_back(move(t2)); //.... //transfer t2 pointee to v2 v1 = move(v2); //..but t1's pointee is still alive. The programmer has not required any value swapping just a "transfer" and I think the programmer expects all previous v1 values should have been destroyed. This has been discussed before, but I agree that we should have a consensus on these issues. Maybe committee members have some guidelines for move semantics.

I'm still looking forward for Boost.Move being scheduled for review.

Still waiting... ;-) Thanks for our comments, Best, Ion

Frank Mori Hess escribió:

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1

On Monday 24 August 2009, Ion Gaztañaga wrote:

...

Thomas Klimpel escribió:

...

I would say that if an automatic "bad smell" detection tool like Coverity would flag the above code as dangerous, it wouldn't be completely wrong. After mx was used via boost::move(mx), it is not supposed to be used again. So I think it would be better to write

vector(BOOST_RV_REF(vector) mx) : base_t(mx) { this->swap(mx); }

which is also less confusing for a human reader. Yes, but that wouldn't be the right thing. When writing move constructors one must first move base classes and then members.

Yes, but the swap swaps both the base members and derived members.

Ooops! Thanks for spotting this.

Wouldn't it look less suspicious if base_t were constructed the same way as in the default vector constructor? In this case moving into the base class can't accomplish anything useful even in principle, since whatever it does will be blown away by the swap. And if a move constructor were added to base_t (detail::vector_alloc_holder doesn't appear to have one) which modified its moved-from argument, it wouldn't cause bugs.

Yes, default constructor would be right, the problem is that an allocator might not have a default constructor. I'll fix this. Thanks, Ion

Ion Gaztañaga wrote:

Thomas Klimpel escribió:

...

So my position is that the user better be prepared to expect that "a = boost::move(b); " can be implemented as "a.swap(b)", because move assignment is about efficiency, and a.swap(b) is often the most efficient implementation of move assignment.

Some additional discussion can be found in this thread:

http://lists.boost.org/Archives/boost/2007/04/119872.php

I was also against clear(), but somehow people convinced me and I added clear() to my classes ;-)

I have read that thread now. I didn't have the impression that this thread really reached a conclusion. I like the idea of Howard Hinnant to examine the (generic) algorithms that work with move semantics to determine how move-construction and move-assignment should behave. However, the example he gives why he prefers the semantics of clear(); swap(); is not a (generic) algorithm but an example with concrete types: Howard Hinnant wrote:

Now consider vector<thread>: vector<thread> v1; ... vector<thread> v2; ... v1 = std::move(v2); After the move assign, I think it best that whatever threads v1 happened to previously refer to, are now busy canceling themselves, instead of continuing to run under v2's ownership. If you want the latter, you've got swap.

If the user wants to ensure that the threads get canceled, he can call "v2.clear();" himself. So the question for me would be whether there are (generic) algorithms (because they can't call x.clear() themselves) that will use more resources if the move-assignment doesn't call x.clear() for them. After all, those (generic) algorithms are the real clients that will call std::move (x) and will have to deal with non-temporary moved from objects.

The programmer has not required any value swapping just a "transfer" and I think the programmer expects all previous v1 values should have been destroyed.

To be honest, it is exactly because of this argument why I don't like the clear() in the assignment operator of vector. The words "The only requirement is that the object remain in a self consistent state (all internal invariants are still intact)." don't forbid to implement move-assignment as swap, so if the programmer expects something incompatible with this implementation, he is relying on undefined behavior. The move-constructor and move-assignment must be implemented for every class that wants to implement move semantics, so it's best not to burden these with unnecessary requirements and expectations. On the other hand, there might be other good reasons to call clear() in the assignment operator, or more generally try to free the resources owned by any no longer needed r-value. The resources left in the r-value won't be reused anyway, because the r-value is either a temporary object that will soon be destroyed or an object that will soon be (move-)? assigned a new value. The typical scenario where it would be helpful if the r-value is left without owning resources is a function that forwards some of its (r-value) argument to another function, but has no information about the forwarded arguments except that the other function requires these as arguments. All resources left in the r-value would be around at least until the function exits, and the might be a waste of resources. So it seems to be a good idea to advise any function that takes an r-value argument to try to free the resources owned by its r-value argument before returning, either by explicitly freeing the resources or by forwarding the r-value to another function (thereby forwarding the responsibility to free the resources owned by its r-value argument).

This has been discussed before, but I agree that we should have a consensus on these issues. Maybe committee members have some guidelines for move semantics.

I found the following http://www.open-std.org/JTC1/sc22/WG21/docs/papers/2002/n1377.htm "The difference between a copy and a move is that a copy leaves the source unchanged. A move on the other hand leaves the source in a state defined differently for each type. The state of the source may be unchanged, or it may be radically different. The only requirement is that the object remain in a self consistent state (all internal invariants are still intact). From a client code point of view, choosing move instead of copy means that you don't care what happens to the state of the source." Both Peter Dimov and Howard Hinnant participated in the above thread. I had the impression that they still consider the above requirements sufficient. However, Howard Hinnant also stated in the above thread "Originally I preferred vector move assignment as just swap(). However I've recently become convinced that this is not the best definition. I now prefer the semantics of clear(); swap();." Even if the advice "Any function that takes an r-value argument should try to free the resources owned by its r-value argument before returning, either by explicitly freeing the resources or by forwarding the r-value to another function (thereby forwarding the responsibility to try to free the resources owned by its r-value argument)." would be present in some important place, the semantics of 'clear(); swap();' for move assignment would still be questionable, because the memory owned by the vector is not freed. So what's my own opinion on this topic now? Trying to free resources owned by r-value arguments before returning is probably a good idea, because it is often what the programmer expects, it improves decoupling of concerns and reduces unexpected surprised when forwarding of r-value arguments. As a consequence, even so implementing the move-assignment operator as a pure swap looks temping at first sight, this is not the best definition. Regards, Thomas

on Mon Aug 24 2009, Thomas Klimpel <Thomas.Klimpel-AT-synopsys.com> wrote:

Ion Gaztañaga wrote:

...

Thomas Klimpel escribió:

...

So my position is that the user better be prepared to expect that "a = boost::move(b); " can be implemented as "a.swap(b)", because move assignment is about efficiency, and a.swap(b) is often the most efficient implementation of move assignment.

Some additional discussion can be found in this thread:

http://lists.boost.org/Archives/boost/2007/04/119872.php

I was also against clear(), but somehow people convinced me and I added clear() to my classes ;-)

I have read that thread now. I didn't have the impression that this thread really reached a conclusion. I like the idea of Howard Hinnant to examine the (generic) algorithms that work with move semantics to determine how move-construction and move-assignment should behave. However, the example he gives why he prefers the semantics of clear(); swap(); is not a (generic) algorithm but an example with concrete types:

Howard Hinnant wrote:

...

Now consider vector<thread>: vector<thread> v1; ... vector<thread> v2; ... v1 = std::move(v2);

After the move assign, I think it best that whatever threads v1 happened to previously refer to, are now busy canceling themselves, instead of continuing to run under v2's ownership. If you want the latter, you've got swap.

If the user wants to ensure that the threads get canceled, he can call "v2.clear();" himself.

I disagree. It's OK if transforming "x" into "move(x)" changes semantics with respect to x. It isn't OK if it changes semantics with respect to objects other than x. In particular, y = x; ===> y = move(x); should not change anything about what happens with y. That's just too capricious.

So the question for me would be whether there are (generic) algorithms (because they can't call x.clear() themselves) that will use more resources if the move-assignment doesn't call x.clear() for them. After all, those (generic) algorithms are the real clients that will call std::move (x) and will have to deal with non-temporary moved from objects.

Here's another example: // C++03 template <class Pair, class T> T& replace2nd(Pair& c, T const& x) { return c.second = x; } pair<thread,thread> threads; ... long_running_operation(replace2nd(threads, new_thread())); // (**) OK, let's move-enable replace2nd: // addtional overload template <class Pair, class T> T& replace2nd(Pair& c, T&& x) { return c.second = move(x); } can't clear there; we don't know that T has a clear(). Now we've just changed the semantics of the line marked (**), since the old thread won't be canceled until the long-running operation is complete. Replace "thread" with "lock" and now you can easily deadlock, because you've messed with tghe locking/unlocking order. I'm pretty sure we can massage this example easily to eliminate even the appearance of move() in the code. Oh, sure: // C++03 template <class Pair, class F> T& replace2nd(Pair& c, F const& f) { return c.second = f(); // f might return an rvalue }

...

The programmer has not required any value swapping just a "transfer" and I think the programmer expects all previous v1 values should have been destroyed.

To be honest, it is exactly because of this argument why I don't like the clear() in the assignment operator of vector. The words "The only requirement is that the object remain in a self consistent state (all internal invariants are still intact)." don't forbid to implement move-assignment as swap,

The other requirement is that user-observable side-effects of assignment from an lvalue must be preserved. If you own a T that's an implementation detail, you can swap it away and nobody's the wiser. If, as in vector<T>, pair<T>, shared_ptr<T>, etc., you have a T that the user can observe, you need to make sure it is destroyed.

so if the programmer expects something incompatible with this implementation, he is relying on undefined behavior.

Huh? How do you reach that conclusion?

On the other hand, there might be other good reasons to call clear() in the assignment operator, or more generally try to free the resources owned by any no longer needed r-value. The resources left in the r-value won't be reused anyway, because the r-value is either a ^--- usually temporary object that will soon be destroyed or an object that will soon be (move-)? assigned a new value.

Exactly. The capacity of a vector can be valuable when assigning. template <class It, class T> void push_front_like(It start, It finish, T const& x) { while (--finish != start) { It prev = finish; --prev; *finish = std::move(*prev); } // If there's enough capacity left in *start, we avoid a // deallocation and an allocation *start = x; }

The typical scenario where it would be helpful if the r-value is left without owning resources is a function that forwards some of its (r-value) argument to another function, but has no information about the forwarded arguments except that the other function requires these as arguments. All resources left in the r-value would be around at least until the function exits, and the might be a waste of resources. So it seems to be a good idea to advise any function that takes an r-value argument to try to free the resources owned by its r-value argument before returning, either by explicitly freeing the resources or by forwarding the r-value to another function (thereby forwarding the responsibility to free the resources owned by its r-value argument).

You can't do any of that if you don't know anything special about the type of the argument.

...

This has been discussed before, but I agree that we should have a consensus on these issues. Maybe committee members have some guidelines for move semantics.

I found the following http://www.open-std.org/JTC1/sc22/WG21/docs/papers/2002/n1377.htm

"The difference between a copy and a move is that a copy leaves the source unchanged. A move on the other hand leaves the source in a state defined differently for each type. The state of the source may be unchanged, or it may be radically different. The only requirement is that the object remain in a self consistent state (all internal invariants are still intact). From a client code point of view, choosing move instead of copy means that you don't care what happens to the state of the source."

Both Peter Dimov and Howard Hinnant participated in the above thread. I had the impression that they still consider the above requirements sufficient. However, Howard Hinnant also stated in the above thread "Originally I preferred vector move assignment as just swap(). However I've recently become convinced that this is not the best definition. I now prefer the semantics of clear(); swap();."

Exactly. I think Howard just didn't know how to phrase the requirement on user-visible side-effects. Credit where due: he's the one who explained this to me.

Even if the advice

"Any function that takes an r-value argument should try to free the resources owned by its r-value argument before returning, either by explicitly freeing the resources or by forwarding the r-value to another function (thereby forwarding the responsibility to try to free the resources owned by its r-value argument)."

would be present in some important place, the semantics of 'clear(); swap();' for move assignment would still be questionable, because the memory owned by the vector is not freed.

I disagree. The capacity of a vector is not part of its value. -- Dave Abrahams BoostPro Computing http://www.boostpro.com

Howard Hinnant wrote:

Using an experimental C++0x compiler and library (you might can do this experiment g++-4.4, I'm not sure) I make this substitution and indeed, I get the same output (remove all of the *.dat files prior to each run). This experimental library has:

template <class _Tp, class _Allocator> inline vector<_Tp, _Allocator>& vector<_Tp, _Allocator>::operator=(vector&& __x) { clear(); swap(__x); return *this; }

Perhaps a dumb question, but isn't the above incorrect under self-assigment (x = move(x))? Is there a "standard way" or "recommended guideline" for handling self-assignment in the move assignment operator, or is this a nonissue? - Jeff

On Sep 7, 2009, at 3:26 PM, Christopher Jefferson wrote:

On 7 Sep 2009, at 20:08, Jeffrey Hellrung wrote:

...

Howard Hinnant wrote:

...

Using an experimental C++0x compiler and library (you might can do this experiment g++-4.4, I'm not sure) I make this substitution and indeed, I get the same output (remove all of the *.dat files prior to each run). This experimental library has: template <class _Tp, class _Allocator> inline vector<_Tp, _Allocator>& vector<_Tp, _Allocator>::operator=(vector&& __x) { clear(); swap(__x); return *this; }

Perhaps a dumb question, but isn't the above incorrect under self- assigment (x = move(x))? Is there a "standard way" or "recommended guideline" for handling self-assignment in the move assignment operator, or is this a nonissue?

I'm actually not 100% positive what the intended semantics of x = move(x) are, but clearly as a QOI issue it should behave.

Most people put:

if(&__x != this)

or something similar in normal assignment operators, adding it also to all move operators seems very sensible.

My hope has been to make x = move(x) undefined behavior. Rationale: The move assignment operator should be fast enough that an extra if- test may adversely impact performance (above the noise level). The philosophy has been that code should be able to assume that if given an rvalue, then it /really/ is an rvalue. And if the client lied by moving an lvalue, then it is up to the client to make sure that lie is really safe. And if the rhs /really is/ an rvalue, then self assignment is impossible. After all, one of the primary motivations for move semantics is performance. So I'd really like it to be as fast as possible. The slower it is, the less important it is. The draft standard could probably use a statement to that effect. Currently the closest thing in there with regards to this issue is [container.requirements.general]/12:

An object bound to an rvalue reference parameter of a member function of a container shall not be an element of that container; no diagnostic required.

-Howard

On Sep 7, 2009, at 4:44 PM, Peter Dimov wrote:

...

My hope has been to make x = move(x) undefined behavior.

I find this a bit hard to justify.

...

Rationale: The move assignment operator should be fast enough that an extra if- test may adversely impact performance (above the noise level).

The problem with making it UB is that y = move(x) now needs to be guarded with if( &x != &y ) everywhere in client code. Sometimes the user will know that x is not y, but is this really the case most of the time?

It has been part of the design for the past 7 years. From N1377 (2002-09-10):

Move semantics will automatically come into play when given rvalue arguments. This is perfectly safe because moving resources from an rvalue can not be noticed by the rest of the program (<em>nobody else has a reference to the rvalue in order to detect a difference</ em>).

Here it is again in N1690 (2004-09-07):

When the argument is an rvalue, the author of the class knows that he has a unique reference to the argument.

The overload taking an rvalue can modify the argument in whatever invariant preserving way it pleases and be guaranteed that the rest of the program will not notice!

I'm currently thinking that we need a Chapter 17 statement along the lines of: The C++ standard library may assume that an rvalue reference represents a unique reference to the bound object. This statement would have been more difficult to write the standardeze for prior to N2831 (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2009/n2831.html ). This would make [container.requirements.general]/12 redundant, and that paragraph could thus be removed. This would also mean that for every class type defined by the standard library with a move assignment operator: C& C::operator=(C&& rhs); This signature may assume that rhs represents the only reference to the object bound to rhs. After all if the library can't assume (for example) that in vector::push_back(T&& t); t is a unique reference to some object, then it can't very well destructively modify that object while appending it to the vector. -Howard

On Sep 7, 2009, at 5:20 PM, Christopher Jefferson wrote:

On 7 Sep 2009, at 21:44, Peter Dimov wrote:

...

...

...

I'm actually not 100% positive what the intended semantics of x = move(x) are, but clearly as a QOI issue it should behave.

Most people put:

if(&__x != this)

or something similar in normal assignment operators, adding it also to all move operators seems very sensible.

This is not enough in general. clear() may end up indirectly destroying the contents of x, even when &x != this. (Although... now that I think of it... this may be a problem with the vector copy assignment as well.)

...

My hope has been to make x = move(x) undefined behavior.

I find this a bit hard to justify.

...

Rationale: The move assignment operator should be fast enough that an extra if- test may adversely impact performance (above the noise level).

The problem with making it UB is that y = move(x) now needs to be guarded with if( &x != &y ) everywhere in client code. Sometimes the user will know that x is not y, but is this really the case most of the time?

Actually thinking about it, I wonder if when this occurs it is a mistake -- should instead a debug mode assert if &x == &y?

x = move(x) actually seems like it would be a logic error. Certainly having now implemented a reasonable amount of rvalue code, including a standard library implementation, I can't think of where it would arise without an error. move(x) implies we do not care about the value of x, but then the assignment means that we do care.

Certainly I'd want to see an assert in debug mode, to catch this, as I imagine errors it causes could be quite subtle.

<nod> Along the same line of thought a debug mode vector/deque::insert (p, T&& t) could detect/assert if &t is within the range of those objects that need to be moved to make room for t. -Howard

on Mon Sep 07 2009, "Peter Dimov" <pdimov-AT-pdimov.com> wrote:

...

Rationale: The move assignment operator should be fast enough that an extra if- test may adversely impact performance (above the noise level).

The problem with making it UB is that y = move(x) now needs to be guarded with if( &x != &y ) everywhere in client code. Sometimes the user will know that x is not y, but is this really the case most of the time?

Most of the time, yeah. Definitely, in most of the generic algorithms. But I'm not sure whether it's true enough of the time. Consider our new generic std::swap: template <class T> void swap(T& x, T& y) { T z = std::move(x); x = std::move(y); // <== uh oh? y = std::move(z); } Do we think swap(x,y) is also an error when x == y? -- Dave Abrahams BoostPro Computing http://www.boostpro.com

On Sep 7, 2009, at 6:03 PM, David Abrahams wrote:

on Mon Sep 07 2009, "Peter Dimov" <pdimov-AT-pdimov.com> wrote:

...

...

Rationale: The move assignment operator should be fast enough that an extra if- test may adversely impact performance (above the noise level).

The problem with making it UB is that y = move(x) now needs to be guarded with if( &x != &y ) everywhere in client code. Sometimes the user will know that x is not y, but is this really the case most of the time?

Most of the time, yeah. Definitely, in most of the generic algorithms. But I'm not sure whether it's true enough of the time. Consider our new generic std::swap:

template <class T> void swap(T& x, T& y) { T z = std::move(x); x = std::move(y); // <== uh oh? y = std::move(z); }

Do we think swap(x,y) is also an error when x == y?

This is a decent argument to not assert in self-move-assignment. But this is also almost certainly harmless: #include <algorithm> #include <iostream> class A { int data_; public: explicit A(int data = 0) : data_(data) {} A(A&& a) : data_(a.data_) {a.data_ = 0;} A& operator=(A&& a) {data_ = a.data_; a.data_ = 0; return *this;} friend std::ostream& operator<<(std::ostream& os, const A& a) { return os << a.data_; } }; int main() { A a(5); std::cout << "Before swap a = " << a << '\n'; std::swap(a, a); std::cout << "After swap a = " << a << '\n'; } Before swap a = 5 After swap a = 5 But if I caught my own code doing a self-swap, yeah, I would treat it as a bug and correct it. For example, every time I've written reverse (which does nothing but swap x and y while x and y move closer to each other in the sequence), I'm careful to break out of the loop before &x == &y. -Howard

On Sep 7, 2009, at 6:50 PM, David Abrahams wrote:

on Mon Sep 07 2009, Howard Hinnant <howard.hinnant-AT-gmail.com> wrote:

...

...

Do we think swap(x,y) is also an error when x == y?

This is a decent argument to not assert in self-move-assignment.

And not to call it undefined behavior. self-std:swapping was allowed in C++03 and I don't think we can break that.

...

But this is also almost certainly harmless:

#include <algorithm> #include <iostream>

class A { int data_; public: explicit A(int data = 0) : data_(data) {} A(A&& a) : data_(a.data_) {a.data_ = 0;} A& operator=(A&& a) {data_ = a.data_; a.data_ = 0; return *this;} friend std::ostream& operator<<(std::ostream& os, const A& a) { return os << a.data_; } };

int main() { A a(5); std::cout << "Before swap a = " << a << '\n'; std::swap(a, a); std::cout << "After swap a = " << a << '\n'; }

Before swap a = 5 After swap a = 5

Yes, it's harmless, but I don't see how it's a "but." I must be missing your point, because that example seems to support my argument.

...

But if I caught my own code doing a self-swap, yeah, I would treat it as a bug and correct it. For example, every time I've written reverse (which does nothing but swap x and y while x and y move closer to each other in the sequence), I'm careful to break out of the loop before &x == &y.

Yes, but if it's reverse's job to avoid that, then it's also swap's job (both are algorithms, neh?) So should we put a self-swap test there?

No, I don't think so. std::swap will only do a self-move-assignment using a moved-from object. Of course vector will never use this definition. But if it did, then: template <class _Tp, class _Allocator> inline vector<_Tp, _Allocator>& vector<_Tp, _Allocator>::operator=(vector&& __x) { clear(); swap(__x); return *this; } would work just fine. clear() is a no-op, and then you swap an empty capacity vector with itself. My point is that I don't want to be told I need to put a if(this != &x) in my move assignment operator. It is a useless performance penalty. Everyone should not have to pay (in terms of lost performance) for someone's bad code just to make sure self-move- assignment does a no-op instead of crashes. If you want to cast an lvalue to an rvalue, that's fine. Just make sure your code logic is such that such a cast is safe. I believe std::swap's use of move assignment is safe because by the time it move-assigns, if it is self referencing, all of your resources have already been moved to a local temp. -Howard

On Sep 7, 2009, at 8:37 PM, David Abrahams wrote:

on Mon Sep 07 2009, Howard Hinnant <howard.hinnant-AT-gmail.com> wrote:

...

std::swap will only do a self-move-assignment using a moved-from object.

You mean the target will be a moved-from object.

<nod>

OK, so?

So the target is resource-less. There are no pre-existing resources the target must get rid of prior to the move-assignment. That means you're not going to delete yourself, even if the move assignment is self-referencing.

...

Of course vector will never use this definition. But if it did, then:

template <class _Tp, class _Allocator> inline vector<_Tp, _Allocator>& vector<_Tp, _Allocator>::operator=(vector&& __x) { clear(); swap(__x); return *this; }

would work just fine. clear() is a no-op, and then you swap an empty capacity vector with itself.

My point is that I don't want to be told I need to put a if(this != &x) in my move assignment operator. It is a useless performance penalty. Everyone should not have to pay (in terms of lost performance) for someone's bad code just to make sure self-move- assignment does a no-op instead of crashes.

And the reason this argument doesn't also apply to copy assignment is...?

swap doesn't use copy assignment. ;-) More seriously because:

I'm currently thinking that we need a Chapter 17 statement along the lines of:

The C++ standard library may assume that an rvalue reference represents a unique reference to the bound object.

This simple statement removes sometimes-expensive error checking that is vanishingly-rarely useful. One can not self-assign a true (non- casted) rvalue. Move semantics only works because the code with the reference to the rvalue can assume that it has the *only* reference to that rvalue. (this is sort of like C99's restrict in reverse)

...

If you want to cast an lvalue to an rvalue, that's fine. Just make sure your code logic is such that such a cast is safe. I believe std::swap's use of move assignment is safe because by the time it move-assigns, if it is self referencing, all of your resources have already been moved to a local temp.

Then what is the coding guideline, exactly? "Make sure it's safe" doesn't qualify.

Agreed. A good guideline might go along the lines of: if you move assign, the target should generally be moved from first. If it is not, ensure that target and source do not refer to the same object. This is not as hard as it sounds: Every single in-place-sequence- permuting algorithm in <algorithm>, including swap, sort, rotate, random_shuffle, partition, nth_element, etc. works by either swapping, or by move-constructing an element out to a local temp, and then move assigning into the "hole" (and then move assigning into the new "hole"). I.e. these algorithms always move assign into a moved-from element. This behavior also includes the container member algorithms for vector/deque insert. An exception to the above statement is those algorithms which "shorten" the sequence: remove and remove_if, and the container member functions erase (for vector and deque). Here the initial move assignment is into a desired-removed but not moved-from element. This algorithm will typically result in the trailing "removed" elements all being moved-from. In these algorithms, there is no chance of move assignment being self referencing (except for a coding error on the part of the author of remove/erase, etc.). -Howard

on Thu Oct 15 2009, Thomas Klimpel <Thomas.Klimpel-AT-synopsys.com> wrote:

I know that this discussion is old, and that a review manager for the review is still missing.

I'll manage the review if nobody else is stepping up.

However, since the article series at "http://cpp-next.com" has now covered quite some ground, I somehow guess that a review manager will be found sooner or later. (More precisely, I guess that the article series on value semantics, moving and all that will soon be finished, and that then a review manager will appear somehow...)

Not sure what C++Next has to do with it, but...

...

It's OK if transforming "x" into "move(x)" changes semantics with respect to x. It isn't OK if it changes semantics with respect to objects other than x. In particular,

y = x; ===> y = move(x);

should not change anything about what happens with y. That's just too capricious.

I'm not sure about this. An alternative would be to transform "x" into "X(move(x))" (move constructor call). At least this solution would be more general than the suggestion to call "v2.clear();".

As http://cpp-next.com/archive/2009/09/your-next-assignment/ points out, generality isn't the point of move---optimization is--- and if you want generality, you can go with X& operator=(X&& rhs) { X(std::move(rhs)) .swap(*this); return *this; }

...

Here's another example:

// C++03 template <class Pair, class T> T& replace2nd(Pair& c, T const& x) { return c.second = x; }

pair<thread,thread> threads; ...

long_running_operation(replace2nd(threads, new_thread())); // (**)

OK, let's move-enable replace2nd:

// addtional overload template <class Pair, class T> T& replace2nd(Pair& c, T&& x) { return c.second = move(x); }

can't clear there; we don't know that T has a clear().

Now we've just changed the semantics of the line marked (**), since the old thread won't be canceled until the long-running operation is complete.

Interesting. I have to admit that I assumed the temporary objects for the arguments to replace2nd would be destroyed before the return value of replace2nd gets used, but this assumption was probably wrong. I guess the C++ standard just says that they get destroyed after the entire expression is evaluated.

Yes

Just a simple question for my education: Does the C++ standard explicitly guarantees that the temporary objects don't get destroyed before the entire expression is evaluated,

Yes

or is the exact moment of the destructor calls simply unspecified?

No <snip>

So I'm left with the impression that there might be quite some good reasons not to implement move-assignment as swap, but no really conclusive counter examples against it. On the other hand, as long as the absence of conclusive counter examples can't be "proved", ruling that move-assignment may generally be implemented as swap will be risky or worse.

I guess we're in agreement, then :-) -- Dave Abrahams Meet me at BoostCon: http://www.boostcon.com BoostPro Computing http://www.boostpro.com

On Oct 17, 2009, at 4:39 AM, Christopher Jefferson wrote:

On 16 Oct 2009, at 02:10, Thomas Klimpel wrote:

...

So I'm left with the impression that there might be quite some good reasons not to implement move-assignment as swap, but no really conclusive counter examples against it. On the other hand, as long as the absence of conclusive counter examples can't be "proved", ruling that move-assignment may generally be implemented as swap will be risky or worse.

Just as one point of information, which I've previously seen used in favour of using swap as move.

While it is true that a move which doesn't swap on (for example) vector<T> is O(n) rather than O(1), due to the need to destruct all the elements, I found in experiments that sorting a range of vector<T> is actually very slightly faster when move-assignment is NOT implemented as swap, so performance isn't even really an argument in it's favour.

There's a theoretical foundation behind this empirical observation. sort is a "resource preserving" permutation (as are many, but not all, algorithms in <algorithm>). I.e. things just get rearranged. Nothing need be constructed or destructed, though such constructions/ destructions may happen as implementation details. swap is the simplest "resource preserving" permutation algorithm, and for the purposes of this argument, is no different from sort: template <class T> void swap(T& x, T& y) { T t(std::move(x)); x = std::move(y); y = std::move(t); } The construction of t will put x into a resource-less state. The move assignment of x is <em>into an object with a resource-less state</ em>. I.e. here it makes no difference if T's move assignment operator is implemented with or without clear(). There is nothing to clear(). This move assignment will put y into a resource-less state. In the last move assignment, it is again into an object with a resource-less state. There is nothing to clear() out of y by the time it is move- assigned into. This pattern: if you move assign into something, it is already resource-less; is just as true for sort as it is for swap. And for unique. And for stable_partition. And for stable_sort. And for inplace_merge. Etc. Thus the cost for clearing the lhs within a move-assignment prior to the transfer of resources is minimal. It is usually a no-op. So why do it? What's the benefit even if the cost is small? Consider std::remove: This is one of the few algorithms in <algorithm> which is not a "resource preserving" permutation. Consider remove({a, b, c, d, e}, c). This will result in the sequence {a, b, d, e, e'}. To achieve this result, the following move assignments will take place: c = move(d); d = move(e); The first move assignment is not into a resource-less c. If move assignment into c doesn't "clear", when do those resources go away? For most resources (such as memory), it doesn't really matter when. The resources will still be owned if move-assignment doesn't aggressively destroy them (probably by e'). But for some resources (threads, file handles, mutexes), it will matter if they live too long. The client calling std::remove may or may not destruct e' "soon". But the client asked std::remove to remove c (and presumably all of c's resources). I do not think it would be a good idea for std::remove to also say: Oh, and you better destruct e' too just to make sure c is really gone. This is the benefit of move assignment "clearing" potentially significant resources aggressively. I believe this benefit outweighs the cost. -Howard

On Oct 17, 2009, at 12:26 PM, Thomas Klimpel wrote:

Howard Hinnant wrote:

...

So why do it? What's the benefit even if the cost is small?

Consider std::remove: This is one of the few algorithms in <algorithm> which is not a "resource preserving" permutation. Consider remove({a, b, c, d, e}, c). This will result in the sequence {a, b, d, e, e'}. To achieve this result, the following move assignments will take place:

c = move(d); d = move(e);

The first move assignment is not into a resource-less c. If move assignment into c doesn't "clear", when do those resources go away? For most resources (such as memory), it doesn't really matter when. The resources will still be owned if move-assignment doesn't aggressively destroy them (probably by e').

But for some resources (threads, file handles, mutexes), it will matter if they live too long. The client calling std::remove may or may not destruct e' "soon". But the client asked std::remove to remove c (and presumably all of c's resources). I do not think it would be a good idea for std::remove to also say: Oh, and you better destruct e' too just to make sure c is really gone.

I was initially looking for an example of this sort to "prove" that "clear" is required, but came to realize that I probably won't find one. (The examples Dave gave me are sufficient to "prove" that "clear" is required, but they are related to subtleties of the C++ language and not inherent in moving itself.)

But why do I think that this example doesn't "prove" that "clear" is required? Consider "remove({a, b, c, d, c}, c)" instead of "remove ({a, b, c, d, e}, c)". This will result in the sequence {a, b, d, d', c}. So the assumption that the client asked std::remove to remove c (and presumably all of c's resources) is simply not true. The client really has to destruct the elements of the tail range to ensure that c is really gone, even if the move-assignment operator calls "clear".

That's a good counter example. I should've prefaced my example with something like: The client has already unique'd his sequence and knows that the desired removed element is not at the end of the sequence. I also made another mistake in my last post: unique is not a "resource preserving" permutation.

...

This is the benefit of move assignment "clearing" potentially significant resources aggressively. I believe this benefit outweighs the cost.

My current understanding is that there are more subtle reasons why move assignment should "clear" potentially significant resources aggressively.

There is still the open point whether self-move-assignment should be forbidden. I wrote in another mail: "On the other hand, as long as the absence of conclusive counter examples can't be "proved", ruling that move-assignment may generally be implemented as swap will be risky or worse." I think the same should apply to self-move- assignment: As long as it can't be proved that forbidding self-move- assignment does no harm, ruling that self-move-assignment is forbidden will be risky or worse.

When move assigning into a resource-less lhs, self move assignment appears to me to be harmless. Note that I do not advocate that move-assignment be generally implemented as swap. I generally advise that move assignment be implemented as: 1. Destroy non-memory resources. 2. Move assign base classes. 3. Move assign data members. 4. Fix up invariants. When 1 and 4 are no-ops, it should be safe to let the compiler default this for you. -Howard

On Oct 17, 2009, at 3:52 PM, Thomas Klimpel wrote:

...

I generally advise that move assignment be implemented as:

1. Destroy non-memory resources. 2. Move assign base classes. 3. Move assign data members. 4. Fix up invariants.

The open question is whether "1." should first check for self- assignment before destroying the non-memory resources. My point was that as long as it can't be proved that omitting the check can't do any harm, omitting the check will be risky or worse. I know that it will be no problem if the object is in an empty state, but what about the case where the object is in a non-empty state? And even if we come to the conclusion that we are allowed to omit the check for self-move-assignment, would we be allowed to assert against self- move-assignment from a non-empty object?

I don't believe the standard should really tell you, much less try to enforce, how you write your move assignment operator. The standard should give you guidelines on what it is supposed to do, and how it might be used within the std::lib, and that's about it. I think it would be safe for the std::lib to promise you that it won't self-move- assign your objects, with the possible exception of you calling std::swap(x, x). If std::reserve called swap(x, x), I would consider that a bug in std::reserve. Yes, you should be allowed to check for self-move-assignement, or assert on it, or whatever is best for your code. You should probably refrain from self-move-assigning std types such as vector. -Howard

2009/8/24 Ion Gaztañaga <igaztanaga@gmail.com>

Gottlob Frege escribió:

...

Is stable_vector somewhat similar to that? Or what is the data structure of stable_vector? (Sorry I haven't dug into the code yet to find out - I think it would be nice to mention it in the docs, even if the implementation is suppose to be hidden and separate from the requirements.) Tony

Joaquín wrote the original stable_vector, read hits post:

http://bannalia.blogspot.com/2008/09/introducing-stablevector.html

the internal structure is a vector of pointers pointing to heap-allocated nodes. The advantage is that iterator are stable (it's a node-based container) and you get random-access using the auxiliar pointer array.

Best,

Ion

So very similar. I just 'allocate' from a vector instead of the heap. Which keeps everything together in memory. ie: I see the standard is now going to have a definition of contiguous: &v[n] == &v[0] + n for all 0 <= n < v.size() To me that is "ordered contiguity". I usually find all I really need is "unordered contiguity": for all 0 <= n < v.size() there exists an m with 0 <= m <= v.size() such that &v[n] == &v[0] + m What should that be called? 'locality' ? Tony

Gottlob Frege escribió:

2009/8/24 Ion Gaztañaga <igaztanaga@gmail.com>

...

Gottlob Frege escribió:

...

Is stable_vector somewhat similar to that? Or what is the data structure of stable_vector? (Sorry I haven't dug into the code yet to find out - I think it would be nice to mention it in the docs, even if the implementation is suppose to be hidden and separate from the requirements.) Tony

Joaquín wrote the original stable_vector, read hits post:

http://bannalia.blogspot.com/2008/09/introducing-stablevector.html

the internal structure is a vector of pointers pointing to heap-allocated nodes. The advantage is that iterator are stable (it's a node-based container) and you get random-access using the auxiliar pointer array.

Best,

Ion

Any thoughts (from anyone?) on my comments about making flat_map's iterators stable? Then it would be a complete replacement to std::map. Tony

Making flat_xxx a template adaptor (like stack or similar) and using stable_vector as the underlying type would make flat_stable_vector a replacement for std::map. Best, Ion

Ion Gaztañaga wrote:

Gottlob Frege escribió:

...

Any thoughts (from anyone?) on my comments about making flat_map's iterators stable? Then it would be a complete replacement to std::map. Tony

Making flat_xxx a template adaptor (like stack or similar) and using stable_vector as the underlying type would make flat_stable_vector a replacement for std::map.

Best,

Ion

I would prefer it as is, but I haven't had a need for iterator stability. I thought the point of a flat_map/flat_set was to be an as-lightweight-as-possible generic associative container, but still with reasonable (O(log n)) lookup time. Perhaps a policy-based approach is possible, or an entirely separate class, e.g., stable_flat_xxx? - Jeff

On 8/24/09, Jeffrey Hellrung <jhellrung@ucla.edu> wrote:

Ion Gaztañaga wrote:

...

Gottlob Frege escribió:

...

Any thoughts (from anyone?) on my comments about making flat_map's iterators stable? Then it would be a complete replacement to std::map. Tony

Making flat_xxx a template adaptor (like stack or similar) and using stable_vector as the underlying type would make flat_stable_vector a replacement for std::map.

Best,

Ion

I would prefer it as is, but I haven't had a need for iterator stability. I thought the point of a flat_map/flat_set was to be an as-lightweight-as-possible generic associative container, but still with reasonable (O(log n)) lookup time.

I think the point was to have a map that could be shared in interprocess memory. At least that was the original idea. But that could now be done with a std::map if we had stateful allocators.

Perhaps a policy-based approach is possible, or an entirely separate class, e.g., stable_flat_xxx?

If you had a stable one, which was this more maplike, should it be called flat_map, and the unstable on be renamed? To sorted_vector or something? By either implementing stableness or renaming, I'm trying to minimize potential misuse. I do understand that stableness isn't the most often used feature, but it might catch some by surprise. Probably not the developer that puts in the flat_map, but by the next one that comes by to use it. Tony

- Jeff

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Tim Blechmann escribió:

On 08/12/2009 11:35 AM, Ion Gaztañaga wrote:

...

Boost.Container

http://svn.boost.org/svn/boost/sandbox/move/libs/container/doc/html/index.ht...

according to [1] boost.container works with stateful allocators. browsing through the boost.container documentation, i didn't find a comment about it, though ...

if this is the case, it would be great to see an example and a section in the documentation on how to use it ... it would be very useful, e.g. for thread-local memory pools ...

Ok. I supports staful allocators because Interprocess allocators are stateful, but I should add a section with an example. Best, Ion

Jeffrey Hellrung escribió:

Hi Ion,

It appears that the value_type of a boost::container::vector<...>::const_iterator is const-qualified. Is this the correct semantics? I was under the impression that, generally speaking, value_type's are stripped of const qualification, but I could very well be wrong.

I think you are right (at least other implementations don't const-qualify value_type). I'll see if this error is also in other containers. Best, Ion

Jeffrey Hellrung escribió:

Hi (again) Ion,

I recently ran into the following issue. I defined a copyable-and-movable class X, and class Y has a member variable of type X and no other member variables. It thus (apparently) happens that Y's auto-generated operator= takes a non-const reference to Y, which I would guess is generally not desired, or at least an operator= overload taking a const reference to Y should be available.

Yes, I recently got the same issue in Interprocess. The standard says that the auto-generated assignment takes non-const argument if any of bases or members' assignment is non-const. Since we generated the non-const version with Boost.Move, assignment operators are not generated with const arguments. In this case the only solution is to avoid compiler-generated assignment operator and write an explicit one. Copy constructor does not suffer this because we rely on RVO to avoid copies. The fact that you need to add *both* const and non-const is strange, in my case, just definining the const version in the higher-level class was enough.

The only point I want to make is that it seems worth mentioning this subtlety (at least it was a subtlety to me) in the move documentation somewhere, and perhaps how to most easily and portably address this (I'm not sure what that would be), as I would expect it to be a not uncommon situation.

Yes, I should warn about this issue.

- Jeff

Ion

David Abrahams escribió:

...

The fact that you need to add *both* const and non-const is strange, in my case, just definining the const version in the higher-level class was enough.

:-(

This was a weakness of all earlier approaches that was fixed by Adobe's library; I *really* do not want to see it broken by Boost. It's not always something you can work around. Consider putting a movable-and-copyable type in std::pair; now constant pairs can't be copied, which breaks everything.

Well, maybe benefits don't outweigh problems... I added that overload so that assignment moves rvalues instead of copying them, but maybe it's a limitation and we should just live with it. Or maybe the library needs a radical change, I really don't know what would be the best approach. Thanks, Ion

David Abrahams escribió:

...

Well, maybe benefits don't outweigh problems... I added that overload so that assignment moves rvalues instead of copying them,

In the first version of the library we had two overloads ("const T &" and "rv<T> &") for copy/move constructor and assignments. But if T& operator=(const T&) is defined then: T produce(); T t; //... //Copy assignment called instead of move-assignment! t = produce(); This is suboptimal for assignments, because a move is replaced with a copy. In theory this also happens for copy-constructors but fortunately RVO is a much better approach and in practice this problem only happens with assignments: //In theory old approach would also call the copy constructor //but nearly all modern compilers use RVO. T t(produce()); That was the state of the library when I first proposed it. Then Klaus Trindle proposed an alternative overload set to catch non-const rvalues using "const rv<T> &" (the additional ovreloads "were rv<T> &" and "T&" --> note that is non-const) and that solves the assignment problem: http://lists.boost.org/Archives/boost/2009/06/153266.php However, this approach requires defining a non-const assignment operator, so we have a new problem: compiler-generated copy assignment takes the argument as non-const T &. So right know I have two options: the old approach that produces suboptimal assignments from non-const rvalues and the newer one that is optimal but hurts the compiler generated copy assignment. We need a third alternative: a new overload set that maintains "const T &" and properly catches non-const rvalues. Best, Ion

On Sep 8, 2009, at 5:52 AM, Ion Gaztañaga wrote:

We need a third alternative: a new overload set that maintains "const T &" and properly catches non-const rvalues.

I think we have it: define one copy assignment operator in terms of swap. Copy elision makes it nearly as fast as a move, and we don't get an operator=(T&) infection in derived classes. I have no problem with requiring move-enablers to supply a swap. -- David Abrahams BoostPro Computing http://boostpro.com

David Abrahams escribió:

On Nov 16, 2009, at 7:46 AM, David Abrahams wrote: m with requiring move-enablers to supply a swap.

In fact, I don't even think we need to do that. This seems to work pretty well:

#define BOOST_COPYABLE_AND_MOVABLE(TYPE)\ public:\ TYPE& operator=(TYPE t)\ { return this->operator=(static_cast< ::boost::rv<TYPE>&>(t)); } \ //

Interesting. This works for some types but it has performance impact in types likevector because they can't reuse previous allocated resource (e.g. capacity). Ion

David Abrahams wrote:

On Nov 16, 2009, at 7:46 AM, David Abrahams wrote:

...

On Sep 8, 2009, at 5:52 AM, Ion Gaztañaga wrote:

...

We need a third alternative: a new overload set that maintains "const T &" and properly catches non-const rvalues. I think we have it:

define one copy assignment operator in terms of swap. Copy elision makes it nearly as fast as a move, and we don't get an operator=(T&) infection in derived classes.

I have no problem with requiring move-enablers to supply a swap.

In fact, I don't even think we need to do that. This seems to work pretty well:

#define BOOST_COPYABLE_AND_MOVABLE(TYPE)\ public:\ TYPE& operator=(TYPE t)\ { return this->operator=(static_cast< ::boost::rv<TYPE>&>(t)); } \ public:\ operator ::boost::rv<TYPE>&() \ { return *reinterpret_cast< ::boost::rv<TYPE>* >(this); }\ operator const ::boost::rv<TYPE>&() const \ { return *reinterpret_cast<const ::boost::rv<TYPE>* >(this); }\ private:\ //

-- David Abrahams BoostPro Computing http://boostpro.com

I think this was similar to what Ion had before, no? Only the user had to explicitly provide the copy assignment (which accepted a by-value parameter). The downside is the double copy of non-movable objects during copy assignment when no copy elision can kick in: copyable_and_movable x(...); copyable_and_movable y(...); x = y; // non-movable stuff in y is copied twice, right? In fact, copyable_and_movable may not even have anything movable in it (if it's templated, e.g., std::pair), so operator=(T) does twice as much work as operator=(const T&) ! The "current" solution (that which is in the sandbox, using BOOST_COPY_ASSIGN_REF) avoids this double copying at the expense auto-generating an operator= that accepts non-const references. Also, as Ion mentioned, currently acquired resources can't be reused if your copy assignment signature accepts by value. That said, I think I'd prefer a by-value operator=, as it seems reasonably close to optimal and gives better semantic behavior regarding auto-generated operator=. Is there some way to generate a (close to) optimal operator= overload for std::pair? I.e., the BOOST_COPYABLE_AND_MOVABLE macro provides one of operator=(const pair&) or operator=(pair) depending on some compiler time boolean value? I'm thinking: template< class T0, class T1 > pair { private: static const bool should_move = T0::should_move || T1::should_move; // obviously not exactly this but the intent should be clear... BOOST_COPYABLE_AND_MOVABLE( pair ) // refers to should_move to decide whether to generate operator=(const pair&) or operator=(pair) public: T0 first; T1 second; }; Something like: template< class T > typename boost::enable_if_c< boost::is_same< T, pair >::value && should_move, pair& >::type operator=(T x); template< class T > typename boost::enable_if_c< boost::is_same< T, pair >::value && !should_move, pair& >::type operator=(const T& x); Is this worth pursuing? Also, the conversion operator to const rv<T>& is a must, as it allows all functions sans T::operator= to be overloaded on const rv<T>&, T&, and rv<T>& to truly capture rvalues by reference. - Jeff

Jeffrey Hellrung escribió:

Hey Ion, it's me again,

I have roughly the following situation: [...] Do you (or anyone else) have any suggestions as to the best (cleanest, easiest to maintain, etc.) way to proceed?

Maybe you should just leave assignment operator with const D & and try an explicit move in assigments. At least assignments will be suboptimal (just like before) but it won't break any code.

Also, I think this might be another "gotcha" to add to the documentation.

Yes, but a good issue to discuss in the mailing is if BOOST_COPY_ASSIGN is really a good idea. Best, Ion

on Wed Sep 02 2009, Jeffrey Hellrung <jhellrung-AT-ucla.edu> wrote:

Hi Ion,

It appears that the value_type of a boost::container::vector<...>::const_iterator is const-qualified. Is this the correct semantics? I was under the impression that, generally speaking, value_type's are stripped of const qualification, but I could very well be wrong.

You are right. -- Dave Abrahams BoostPro Computing http://www.boostpro.com