On 05/07/2010 02:19 AM, Ion Gaztañaga wrote:

On 07/05/2010 5:36, OvermindDL1 wrote:

...

Getting the library ===================

The latest version of this library may be downloaded from

SVN: http://svn.boost.org/svn/boost/sandbox/move/

and the docs may be viewed here

Docs: http://igaztanaga.drivehq.com/libs/move/doc/html/index.html

and do note, this library is header-only and is designed to be absolutely generic to be useful in all areas of C++ programming.

I've also packed and uploaded code and docs in a zip file for those that prefer direct download:

http://igaztanaga.drivehq.com/move_review.zip

Ion

Ion, do you have the built docs (html) somewhere floating around in the sandbox repo? I can't locate them (which might be because they don't exist, and they would be convenient to have)... - Jeff

OvermindDL1 wrote:

It's my pleasure to announce that the review of Ion Gaztañagas' Move library starts today, May 10th and lasts until May 24th, 2010, unless an extension occurs.

Many thanks to Ion for his great work and thanks also to OvermindDL1 for managing this review.

- What is your evaluation of the design?

Compared to the Adobe Move Library, Boost.Move seems to be much more complicated. (I'm referring to the version from ASL 1.0.42 here. Earlier versions like that from ASL 1.0.34 are a different story. So if my opinion surprises you, this might explain it.) So we have to ask what it offers over the old library that could justify this added complexity. 1) The old library doesn't support "Movable but Non-Copyable" types. This makes the old library useless for many important use cases (see Boost.Thread for example). 2) The old library is not able to exploit true rvalue references for compilers that implement them. 3) The old library doesn't offer an emulation of std::forward (for "Constructor Forwarding"). 4) The old library implements adobe::move() and automatically move assigns from temporaries. While (2) and (3) are nice to have, they are less important than (1) and (4). But since (2) is one of the design goals of Boost.Move, it is also very important. The essence of the design of the Adobe Move Library is template <typename T> struct move_from { explicit move_from(T& x) : source(x) { } T& source; }; template <typename T> T move(T& x, typename move_sink<T>::type = 0) { return T(move_from<T>(x)); } template <typename T> T& move(T& x, typename copy_sink<T>::type = 0) { return x; } and the essence of the design of Boost.Move is namespace boost { template <class T> class rv : public T { rv(); ~rv(); rv(rv const&); void operator=(rv const&); }; } #define XXX_MOVABLE_XXX(TYPE) ... \ operator ::boost::rv<TYPE>&() ... \ operator const ::boost::rv<TYPE>&() const ...\ These conversion operators will kick in when no other better matching overload is available. The trick here is that temporaries can't match against "TYPE&", so as long as no "const TYPE&" or "TYPE" overload is available, the conversion operators will kick in. It seems impossible to achieve (1) or (2) with this design of the Adobe Move Library, while the design of Boost.Move up to this point doesn't seem to exclude to offer any of the functionality provided by the Adobe Move Library. Note that the design up to this point already achieved (1). In order to achieve (2), Boost.Move has to define some suitable macros, and document their intended usage. It seems to me that the existing implementation provides (4), but the existing documentation doesn't clearly state this. So the basic design is sound (I don't expect it to change), but it looks as if Boost.Move currently suffers from a proliferation of macros. Also the documented conventions how to use the provided functionality is probably not yet in its final form. 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 is your evaluation of the implementation?

It's good. There are many examples and tests. And I should add that "Constructor Forwarding" is a nice bonus. However, are we sure that it is a good idea to call this functionality boost::forward()? Do we know whether it is impossible to implement Boost.Move in standard conform C++03 ("strict aliasing rule")? Also, I think that the definition of BOOST_COPYABLE_AND_MOVABLE and BOOST_MOVABLE_BUT_NOT_COPYABLE is correct in that it ends with "private:". This enables the user to put it at the top of the class declaration, and have the least surprise in case he puts it somewhere else (if he forgets the "public:" after it, the worst that can happen is a compile error).

- What is your evaluation of the documentation?

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. It starts with things like "holder& operator(holder &)", then continues with things like Daniel James wrote:

I think there's a typo in 'libs/move/doc/html/move/two_emulation_modes.html', under 'optimized mode' you write 'needs to define a copy constructor for copyable_and_movable' but the code is an assignment operator.

and becomes unacceptable when it states "cm = copyable_and_movable(); //temporary object is COPIED", without suggesting a proper workaround (which would be to simply follow the approach of the Adobe Move Library for copyable_and_movable classes), not to mention explaining the possible trade-offs that the user can choose from. There is also the question of the role of the documentation for this specific library. I don't think that it should say much more about the requirements for movable types at the moment. Instead, it should try to reference the currently available documentation for this topic, for example the "Value Semantics" series at <http://cpp-next.com>, but also the most relevant places from the upcoming standard.

- What is your evaluation of the potential usefulness of the library?

Extremely useful, as we all agree :)

- Did you try to use the library?

Some time ago, I played around with an older version of it: <http://lists.boost.org/Archives/boost/2009/09/156034.php>

With what compiler? Did you have any problems?

gcc 4.3.2 and MSVC-9.0express No problems. I noticed that templates are the domain were the most important differences to true rvalue semantics show up. But Boost.Move is nevertheless well suited for templates, as the examples and Boost.Container show.

- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?

A week about six month ago and two days now.

- Are you knowledgeable about the problem domain?

I use "swap" since a long time to work around the missing move semantics in places where it is important. I have reviewed quite some documentation about move semantics (including the Adobe Move Library) since I tried to review Boost.Move six month ago.

And finally, every review should answer this question:

- Do you think the library should be accepted as a Boost library?

Yes, but only conditionally on that the documentation in "two_emulation_modes.html" gets fixed. I don't think that anything in the core implementation needs to be changed, and I also have no problems with the current organization of the macros. However, I think that some changes to the macros will occur when "BOOST_MOVE_OPTIMIZED_EMULATION" gets removed (as planed by Ion). It didn't escape me that Ion wants the reviewers to decide whether they prefer the "Optimized mode" or the "Non-optimized mode". In my opinion, the limitation of the "Optimized mode" is not acceptable for certain classes, especially the containers from Boost.Container. But the way the "Non-optimized mode" mode is presented in the documentation with the implication that it seems to be unable to handle move assignment from temporaries for copyable_and_movable would also be hard to accept. However, I believe this is just a documentation issue, and it would work if the user would implement the assignment operator so that it takes its argument by value. I wouldn't object to give the users that want to have the extra bit of performance the possibility to use the "Optimized mode", but the "Non-optimized mode" must stay available, and the macro "BOOST_MOVE_OPTIMIZED_EMULATION" should be removed or replaced by something less intrusive. Regards, Thomas

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

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

Jeffrey Lee Hellrung wrote:

Thomas Klimpel wrote:

...

All that is needed to to wrap its type declaration with the BOOST_MOVE_ASSIGNMENT_WRAPPER macro. [snip]

Do you find this preferable to simply explicitly defining the copyassignment operator in the enclosing class?

The enclosing class might simply want to use the compiler generated copy constructor and copy assignment operator, for maintenance reasons. Another point is the behavior on compilers with rvalue references, as one of the goals of boost.move is to allow to write code that works on compilers with and without rvalue references, without scarifying the performance on new compilers for the compatibility with old compilers. Apart from that, you are right in that the enclosing class could simply define TYPE& operator=(TYPE t) and be freed of all problems, without using the BOOST_MOVE_ASSIGNMENT_WRAPPER macro. But as we are discussing the question whether classes like boost::container::vector should use the optimized move emulation, the exact impact on the enclosing class is quite important.

...

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.

Is this a position based on technical semantics, rather than practical semantics? Because I believe even the non-optimized emulation should effectively move-assign from temporaries via RVO...

If it were not practical semantics, it wouldn't be a serious regression. The practical problem is the following: If the "move enabled" class defines "TYPE& operator=(const TYPE& t)", then the "TYPE& operator=(BOOST_RV_REF(&) t)" overload won't kick in for move assignment from temporaries on C++03 compilers, as "TYPE& operator=(const TYPE& t)" is a better match. It also cannot define a "TYPE& operator=(TYPE t)" overload in addition to "TYPE& operator=(const TYPE& t)" (this would be ambiguous), so RVO can also no longer kick in. The alternative is to just define "TYPE& operator=(TYPE t)", but Ion doesn't like this for classes like boost::container::vector, as this copy assignment operator cannot reuse existing resources. So his proposed non-optimized mode sacrificed move-assign from temporaries on C++03 compilers, to avoid non-optimal code on new compilers in C++0x mode. Regards, Thomas

Jeffrey Lee Hellrung wrote:

Ah, okay, I was thinking of the distinction between the 2 move emulation modes described in the proposed Boost.Move documentation, neither of which use a copy assignment operator with signature operator=(const T&).

The description of the non-optimized mode in <http://igaztanaga.drivehq.com/libs/move/doc/html/move/two_emulation_modes.html> says "Users can use the usual copy-assignment or the compiler-generated one". This is exactly the copy assignment operator with signature "T& operator=(const T&)". The text goes on to explicitly say "But this emulation mode does not optimize the non-const rvalue move assignment example". This is what I mean by "breaking move-assignment from temporaries for the non-optimized mode".

By "exact impact", you mean the "recommended resolution" to define, implicitly or explicitly, a standard copy assignment operator in the enclosing class...?

No, I mean the problems that will impact the enclosing class in profound ways. As long as there are reasonable solutions to these problems (having more than just one solution won't hurt), everything should be fine.

If I understand correctly, optimal move emulation for boost::container::vector is the best choice if we ignore the effect on (future) enclosing classes. Are you/we necessarily against just going with this, documenting the fact that enclosing classes will have to deal with the non-standard compiler-generated copy assignment operator (either by defining it explicitly, perhaps conditionally; or wrapping member vars in a wrapper, also perhaps conditionally)?

I'm not necessarily against it. But I would like to hear Ion's opinion on this.

We don't have any backwards compatibility concerns, since boost::container::vector is not actually released yet, right?

Yes and no. If I understand it correctly, boost::container::vector will be the successor of boost::interprocess::vector (= boost::interprocess_container::vector). And these interprocess containers are meant as replacements for stl containers with extended allocator support, as required for boost.interprocess. Changing the behavior of the copy assignment operator might make them less suitable as replacements for stl containers. Requiring wrapping members into a macro or applying some other "purely syntactic changes" should be OK on the other hand. Regards, Thomas

On 5/31/2010 4:55 AM, Thomas Klimpel wrote:

Jeffrey Lee Hellrung wrote:

...

Ah, okay, I was thinking of the distinction between the 2 move emulation modes described in the proposed Boost.Move documentation, neither of which use a copy assignment operator with signature operator=(const T&).

The description of the non-optimized mode in <http://igaztanaga.drivehq.com/libs/move/doc/html/move/two_emulation_modes.html> says "Users can use the usual copy-assignment or the compiler-generated one". This is exactly the copy assignment operator with signature "T& operator=(const T&)". The text goes on to explicitly say "But this emulation mode does not optimize the non-const rvalue move assignment example". This is what I mean by "breaking move-assignment from temporaries for the non-optimized mode".

This is what I get for failing to properly review the documentation. I seem to remember that in previous discussions of the library, non-optimized move emulation used an operator=(T), so that at least rvalues were properly captured in practice. Now the terminology has changed (at least for me :/ ) - Jeff

Thomas Klimpel wrote:

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:

[snip]

As implemented above, constructor forwarding is still missing, but as boost.move also offers constructor forwarding functionality, it should be possible to fix this.

Now that the results for the "constrained value" review finally got announced, I said to myself that I should at least try to finish this wrapper for the optimized move mode. Find attached the updated wrapper and some tests for the wrapper. The tests could still be improved, but after updating the wrapper, I'm no longer sure that the optimized move mode will be needed at all, because

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.

Perhaps allowing specific movable classes to break move-assignment from temporaries is not too bad after all. Of course, this implies that generic code must write generic_movable = boost::move(Create_GenericMovable()); instead of generic_movable = Create_GenericMovable(); and hence can't benefit from RVO here (and I'm not sure whether the code will still compile when Create_GenericMovable() returns a const reference instead of a value), but will this missed RVO opportunity here be really important? I initially though it was, but I think I was wrong. I tried to write a simple move enabled class in pure C++0x. In order to see anything related to move semantics at all, the simple class has one pointer member and one "copy and movable" member. Here it is (also attached as "simple_class_c++0x.cpp"): #include <utility> class simple_impl {}; class simple_cm {}; class simple_class { typedef simple_class self_t; simple_impl* p; simple_cm m; public: ~simple_class() { if(p) delete p; } simple_class() : p(0), m() {} simple_class(const self_t& cr) : p(0), m(cr.m) { if(cr.p) p = new simple_impl(*cr.p); } simple_class(self_t&& rr) : p(rr.p), m(std::move(rr.m)) { rr.p = 0; } void swap(self_t&& rr) { if (this != &rr) { if (p) delete p; p = rr.p; rr.p = 0; m = std::move(rr.m); } } self_t& operator=(self_t&& rr) { swap(std::move(rr)); return *this; } self_t& operator=(self_t s) { swap(std::move(s)); return *this; } }; I was quite disappointed when I found out that the move assignment operator can't be used, because "xxx = std::move(yyy);" just triggers the error message that the assignment operator is ambiguous. So I changed the copy assignment operator from "self_t& operator=(self_t s) { swap(std::move(s)); return *this; }" to "self_t& operator=(const self_t& cr) { swap(self_t(cr)); return *this; }". Now I wondered whether the class has become less efficient because of that change. The efficiency is still the same, because RVO would only kick in for temporaries, and these are now handled just as efficient by the move assignment operator. But this probably also means that generic_movable = boost::move(Create_GenericMovable()); won't be less efficient than generic_movable = Create_GenericMovable(); because RVO wouldn't gain any efficiency here. I took the freedom to write the assignment operators in terms of "swap" taking its argument by rvalue reference. This "swap" function is actually not my invention, but the new standard offers it for its container classes. It would probably be less confusing to implement a separate function " void move_assign(self_t&& rr)" (identical to the current swap function), and implement swap and the assignment operators in terms of this function: void swap(self_t&& rr) { move_assign(std::move(rr)); } self_t& operator=(self_t&& rr) { move_assign(std::move(rr)); return *this; } //self_t& operator=(self_t s) { move_assign(std::move(s)); return *this; } self_t& operator=(const self_t& cr) { move_assign(self_t(cr)); return *this; } The difference between "move_assign" and the move assignment operator is that "move_assign" doesn't return a reference to "*this", and that it won't render the copy assignment operator ambiguous. The interesting this about "move_assign" is that even when we implement the assignment operator as taking its argument by value (so that we don't need an assignment operator taking its argument be rvalue reference), we still want to have "move_assign", because it will normally be more efficient than a simple swap. So one of the non-optimized modes of Boost.Move (the one where the copy assignment operator can't reuse resources) could define #if !defined(BOOST_NO_RVALUE_REFERENCES) #define BOOST_COPYABLE_AND_MOVABLE(TYPE)\ ... TYPE& operator=(const TYPE& cr) { return operator=(TYPE(cr)); } #else #define BOOST_COPYABLE_AND_MOVABLE(TYPE)\ ... TYPE& operator=(TYPE s) { return operator=(boost::move(s)); } #endif and the user would just have to supply the definition of self_t& operator=(BOOST_RV_REF(self_t) rr) Another non-optimized modes of Boost.Move (the one currently documented by Boost.Move, where move assignment from temporaries only works with rvalue references) would be to just let the user define self_t& operator=(BOOST_RV_REF(self_t) rr) and self_t& operator=(const self_t& cr) like he would have to do when using pure C++0x. Hope this clarified things a bit. Regards, Thomas

Dave Abrahams wrote:

Thomas Klimpel wrote:

...

self_t& operator=(self_t&& rr) { swap(std::move(rr)); return *this; }

I'm really surprised to see rvalue references in this code. Aren't you talking about move _emulation_ here? If you have rvalue references, why not just use real move semantics?

That's probably a good question. On the one hand, I intended (i.e. I tried) to use real move semantics for simple_class. On the other hand, the reason I presented simple_class to the list at all really is closely related to "not" using real move semantics. Let's assume I would have written the assignment operator as self_t& operator=(self_t s) { swap(*this,s); return *this; } and renounced on implementing an explicit move assignment operator (for example because the compiler complained about operator= being ambiguous). Then writing (i) simple_class_instance = boost::move(simple_class()); would be less efficient than (ii) simple_class_instance = simple_class(); because for (i) both the default constructor and the move constructor of simple_class will be called, but for (ii) only the default constructor of simple_class gets called. This is the point where I wished to have an explicit move assignment operator in simple_class. In addition, I realized that I could even improve the efficiency of my copy assignment operator by using the move assignment operator instead of swap: self_t& operator=(self_t s) { return operator=(std::move(s)); } OK, this doesn't compile (operator= is ambiguous), but self_t& operator=(const self_t& cr) { return operator=(self_t(s)); } compiles and has exactly the same effect, as explained in my previous mail. The important consequences for move _emulation_ are that there is a reasonable simple way to write the assignment operator such that "simple_class_instance = boost::move(simple_class());" won't be less efficient than "simple_class_instance = simple_class();". This statement remains true even for more efficient implementations of "self_t& operator=(const self_t& cr)". So breaking automatic move assignment from temporaries is really a sensible option. Now for using Boost.Move without rvalue references, things are a bit different, because self_t& operator=(self_t s) { return operator=(boost::move(s)); } self_t& operator=(BOOST_RV_REF<self_t> rr) { ...; return *this; } compiles without error and does exactly what it should, while self_t& operator=(const self_t& cr) { return operator=(self_t(s)); } self_t& operator=(BOOST_RV_REF<self_t> rr) { ...; return *this; } will probably compile (the compiler might issue a warning about a non-terminating recursion), but is badly broken. The consequence for Boost.Move is that 3 possible move emulation modes are available, each with its own drawbacks when used without true rvalue references: a) A mode that allows reusing existing resources in the copy assignment operator and automatically move assigns from temporaries. However, it modifies the signature of automatically generated copy assignment operators of classes containing such a class in an unwanted way. b) A mode that allows reusing existing resources in the copy assignment operator, but doesn't automatically move assigns from temporaries. c) A mode that doesn't allow reusing existing resources in the copy assignment operator, but automatically move assigns from temporaries. When used with true rvalue references, the limitations of modes a and b go away. Regards, Thomas

On 25/05/2010 18:06, Jeffrey Lee Hellrung, Jr. wrote:

We could provide a macro that translates rvalues into emulated rvalue references (if movable) or const references (if not movable). Unfortunately, this kills by-value parameter optimizations in C++03 :/

Sorry, but I don't get this, could you elaborate a bit more? Best, Ion

On 25/05/2010 23:15, Jeffrey Lee Hellrung, Jr. wrote:

If you're writing a generic algorithm that invokes a function (perhaps one given to the algorithm as a parameter) which happens to return an rvalue, and you want to ensure that this rvalue gets captured as an rvalue reference, you can wrap the function call in a macro that will ensure such a translation takes place in C++03. That is,

Ok, thanks for the explanation, this sounds useful. Best, Ion

On Tue, May 11, 2010 at 12:48 AM, Jeffrey Lee Hellrung, Jr. <jhellrung@ucla.edu> wrote:

Second, I have to admit that I do have some reservations with this incantation of the library.  I will be trying to post multiple comments over the next couple days, as I think of things and get around to expressing my ideas.

In my opinion, *the* primary goal of this library is to provide a standardized convention by which data structures and algorithms within and outside of boost can make use of move semantics in c++03 (there is already such a convention in c++0x).  All the macros, moving and forwarding functions, move iterators, etc., are, I believe, secondary to introducing boost::rv<T>& and what that little class is suppose to represent and how T should interoperate with it.  What I would like to see are a set of requirements for a type to be MovableCopyable, or MovableNoncopyable.  I'd imagine this would essentially consist of a conversion operator to boost::rv<T>&, constructor accepting a boost::rv<T>&, an assignment operator accepting a boost::rv<T>&, and the semantics associated with these member functions.  This means, for example, that, as an author of some class, I am not restricted to using the macros provided by this library to enable move emulation, or the forwarding functions this library provides, or whatever else, if they don't meet my requirements.  For example, one may want to only conditionally enable move emulation, depending on whether its member objects are movable or not.  Why might one not want to enable move emulation?  If you enable optimal emulation, you wind up with a class that doesn't play well with enclosing classes that don't explicitly define an assignment operator.  Minimizing the proliferation of such classes could be a good idea.  Also, if you're just gonna define copy assignment in terms of a copy-and-move or a copy-and-swap, there's no point in using optimal move emulation, since your copy assignment operator may as well accept by value anyway.  I'm hoping these considerations might convince you that there is no "one size fits all" macro to enable move emulation.  I believe an itemization of the nitty-gritty requirements that a class must satisfy to be move-aware will encourage alternative, more targeted methods to enable move emulation while remaining inter-compatible with each other.

I had similar thoughts pass through my mind, I look forward to if you have a design style in your mind for later. On Tue, May 11, 2010 at 12:48 AM, Jeffrey Lee Hellrung, Jr. <jhellrung@ucla.edu> wrote:

Regarding the macro BOOST_MOVE_OPTIMIZED_EMULATION: I think this is a bad idea, but perhaps I'm missing something.  It seems like as soon as you mix in some move-enabled class into your code that *must* use the non-optimal move emulation (either by design, or through its interaction with other enclosing classes that fail to explicitly define a copy assignment operator), *all* your move-enabled classes will have to use the "safe" (i.e., non-optimal) move emulation.  The only serious use case I see is when BOOST_MOVE_OPTIMIZED_EMULATION is always left undefined.  Am I missing something?  If not, then I think we should either 1) offer *only* the "safe" option, for simplicity; or 2) offer both options as separate move emulation enabling macros.

Very good point, perhaps having both macros would provide the best power, any thoughts anyone else? On Tue, May 11, 2010 at 12:48 AM, Jeffrey Lee Hellrung, Jr. <jhellrung@ucla.edu> wrote:

Hopefully this will be something to at least get discussion started.

A little slow due to BoostCon no doubt, but there are plenty of us that are still stuck away, so please, anyone around, do test and give input about the interface, use, efficiency, etc...

On 5/12/2010 3:16 AM, Ion Gaztañaga wrote:

On 11/05/2010 8:48, Jeffrey Lee Hellrung, Jr. wrote:

...

In my opinion, *the* primary goal of this library is to provide a standardized convention by which data structures and algorithms within and outside of boost can make use of move semantics in c++03 (there is already such a convention in c++0x).

The aim is also to provide a portable syntax between C++0x and C++03 compilers, that's why I consider the macros necessary. I would like to offer a syntax any novice could easily understand.

My intention is not to exclude this aim, I only consider it secondary to defining an agreed-upon protocol for data structures and algorithms to interoperate in a move-aware fashion within c++03.

I also understand your position when you say:

...

This means, for example, that, as an author of some class, I am not restricted to using the macros provided by this library to enable move emulation, or the forwarding functions this library provides, or whatever else, if they don't meet my requirements.

but I, as a writer of an algorithm, would like to know how the emulation works and what should I expect when assigning temporary copyable and movable classes. If the performance depends on the emulation used by the author, maybe I can't design the algorithm optimal for both C++0x and C++03.

Precisely why the requirements of a movable type should be explicitly defined, independent of how the move emulation is superficially effected in the code (i.e., which macro you use). As long as T is efficiently constructable and assignable from rvalues and (emulated) rvalue references (rv<T>& or T&&), the algorithm shouldn't care how the move emulation is actually effected within T.

...

I believe an itemization of the nitty-gritty requirements that a class must satisfy to be move-aware will encourage alternative, more targeted methods to enable move emulation while remaining inter-compatible with each other.

As long as they are compatible, there is no problem, but I think having a C++0x-like syntax for move emulation is positive. Maybe both approaches should be available, the macro-based one for average users without forbidding the use of alternative approaches.

I would just like some confidence in knowing what it means to be "compatible". As it stands now, today we'll use rv<T>& to emulate an rvalue reference, and tomorrow you might change the code to use sw<T>& to emulate an rvalue reference, or something even more radical. Who knows? There's (currently) nothing in the interface of (proposed) Boost.Move to suggest that the emulation machinery is nothing more than an implementation detail, which is problematic if code tries to enable move emulation in some alternative way, e.g., conditionally. Don't get me wrong, *some* standard macro should be supplied that enables move emulation with the same syntax in both c++03 and c++0x (and, based on the arguments in my previous email, this should be, at least, the "safe"/"friendly" version, and, optionally, the "optimal" one as well). Like you said, I think, at the same time, the emulation machinery should be exposed in the interface to allow for alternative approaches.

Thanks for your comments, best,

Ion

- Jeff

On 5/14/2010 1:19 AM, Ion Gaztañaga wrote:

On 12/05/2010 17:37, Jeffrey Lee Hellrung, Jr. wrote:

...

My intention is not to exclude this aim, I only consider it secondary to defining an agreed-upon protocol for data structures and algorithms to interoperate in a move-aware fashion within c++03.

Ok, perfect, so what do you think that protocol should look like? Just trying to get which requirements would be essential for different reviewers and try to merge them.

It seems, to me, like the requirements for a type T to be move constructable are: - T is implicitly convertible to rv<T>& (this is a non-const conversion, and the rv<T>& references the same object as the "from" object) - T has a constructor which can accept a rv<T>&; this constructor is the move constructor, which has move semantics. Refining move constructable would be the concept of move assignable: - T has an assignment operator accepting a rv<T>&; this assignment operator is the move assignment operator, which has move semantics. Some additional requirements that I'm not sure how to categorize, but may be desirable, are: - T has an assignment operator that moves from rvalues (assuming RVO; thus, this could be T::operator=(T) or T::operator=(rv<T>&)). - const T is explicitly convertible to rv<T> const& (*). Requirement (*) allows one to overload functions to capture rvalues of type T: void f(T& x) { ...copy x... } void f(rv<T> const& x) { ...copy x... } void f(rv<T>& x) { ...move x... } However, this prevents one from passing an object implicitly convertible to T, so I think a better recommendation for such overloading is something like template< class U > typename enable_if< is_convertible< U&, const T& > >::type f(U& x) { ...copy static_cast< const T& >(x)... } template< class U > typename disable_if< is_same< U, T > >::type f(const U& x) { ...copy static_cast< const T& >(x)... } void f(rv<T>& x) { ...move x... } Even though this is more complicated, it is more consistent with what one would get in C++0x, where implicit conversions to T when calling f would be allowed: void f(const T& x) { ...copy x... } void f(T&& x) { ...move x... } So, to be precise, I'm not sure that (*) should necessarily be encouraged. What do you think of that list so far?

...

Precisely why the requirements of a movable type should be explicitly defined, independent of how the move emulation is superficially effected in the code (i.e., which macro you use). As long as T is efficiently constructable and assignable from rvalues and (emulated) rvalue references (rv<T>& or T&&), the algorithm shouldn't care how the move emulation is actually effected within T.

Yes, algorithms should just need that, and a few functions (move(), forward()) to do their work. They shouldn't care about macros or implementation details.

...

I would just like some confidence in knowing what it means to be "compatible". As it stands now, today we'll use rv<T>& to emulate an rvalue reference, and tomorrow you might change the code to use sw<T>& to emulate an rvalue reference, or something even more radical. Who knows? There's (currently) nothing in the interface of (proposed) Boost.Move to suggest that the emulation machinery is nothing more than an implementation detail, which is problematic if code tries to enable move emulation in some alternative way, e.g., conditionally.

Ok. I don't know if someone will find a better emulation implementation. If we believe that we know enough C++03 to say rv<T> is the beste possible approach, there is no problem in pushing this outside the implementation detail. But we are ties with this implementation forever, and we should be aware of that limitation.

Of course; that's the unfortunate consequence of exposing rv<T> as part of the interface of (proposed) Boost.Move :( The emulation is very good, and I think its shortcomings are impossible to rectify in C++03. One thing that Boost.Move cannot do, and which I think is impossible in C++03, is capture rvalues as templated (emulated) rvalue references: template< class T > void f(rv<T>& x) will only capture explicitly created rvalue references. This makes forwarding more complicated and more restrictive. I can't think of any other "hard" restrictions on Boost.Move, though, i.e., things you can do in C++0x with rvalue references that you can't do with Boost.Move with sufficient machinery. Can you think of anything else? Perhaps we should consider a survey of past and present move emulation strategies to see how this one stacks up against it. This might also be prudent to include in the documentation. I know that Adobe has a move library that I studied some time ago; do you know of any others? - Jeff

On 5/14/2010 4:32 AM, Ion Gaztañaga wrote:

On 12/05/2010 22:08, Jeffrey Lee Hellrung, Jr. wrote:

...

I don't think, as implemented, is_movable works for non-class types (at least that used to be the case, and I think the only change in the implementation has been the introduction of a boost::mpl::bool_). I think for non-class types T, at least in c++03, is_movable<T> should evaluate to false without causing a compiler error.

Well, it shouldn't cause any error, is_convertible should lead to false:

//This works in MSVC 7.1 bool mv = ::boost::is_movable<int>::value;

Hmmm...yes, you're right. I seem to remember that, at some point, it looked like is_movable was trying to instantiate rv< int > (and hence complained about deriving from int), but I don't have the context anymore... :/

...

I don't recall seeing anything concerning has_nothrow_move other than a short description about how it defaults to false and could hurt performance if not specialized. Can you explain the intended use for has_nothrow_move? Is this related to N2983 [1]? Speaking of which, do you have any idea what the status of throwing move constructors is as far as the c++ committee is concerned, whether you agree with the resolution provided by N2983, and if so, have you thought about providing move_if_noexcept (or something similar)?

Since this has been a moving target until now, I haven't seriously thought about emulating those proposals, but I do expect to work on them in the future, when they are completely definitive. Regarding performance gains with has_nothrow_move, you can optimize away some exception rollback actions in container internals if you know that moving objets will not throw. And to use some advanced memory allocation strategies, like forward buffer expansion, you need this nothrow guarantees because some containers, like vector, require strong exception guarantees for some operations (like push_back).

Right, I understand the optimization considerations. I'm torn whether has_nothrow_move is appropriate to include at this juncture. About all I can say is that it *currently* lacks sufficient explanation in the documentation as to its purpose. As you said, throwing move constructors has been a "moving target". I'm hoping someone with more familiarity on the subject can step in and advise.

...

If has_nothrow_move turns out to be widely used, it may be a good idea to package the "no-throw-ness" of the move constructor within a move emulation enabling macro (ENABLE_NOTHROW_MOVABLE), e.g., by defining a typedef that has_nothrow_move can detect, or maybe a boolean static constant within the class indicating whether the move constructor throws, so that you can make the "no-throw" property conditional on the "no-throw-ness" of member objects. In any case, one step at a time...

The good part of template specializations is that you don't need to instantiate the class to check if the class has that property, and this is important specially when defining recursive data types like recursive containers:

class recursive { vector<recursive> leafs; };

Yes, I agree, it is certainly convenient to be able to use has_nothrow_move<T> even if T is incomplete. I still don't like the idea of forcing users to specialize has_nothrow_move when it seems like the *common* case is non-throwing move constructors...hmmmm...

...

As far as the utility of is_movable in c++0x, it looks like it's only used in uninitialized_move and uninitialized_copy_or_move. This doesn't seem like enough justification to provide it for c++0x, since uninitialized_{copy_or_}move *could* be implemented without it just fine. Furthermore, is_movable will only be true in c++0x if a movable type has a nested typedef boost_move_emulation_t, which seems to exclude, e.g., STL data structures. I would think a simpler course of action would be to relegate is_movable to strictly c++03...

Yes, that might be an option. I do use it in containers for some sfinae tricks, but I think it's only for C++03 code. It seems that in C++0x it's not easy to know if a class really has a rvalue reference overload constructor (or at least, it wasn't some time ago, maybe things have changed in recent proposals-drafts), so it might be possible to limit it to C++03.

I think that would be prudent. - Jeff

On 5/15/2010 4:26 PM, vicente.botet wrote:

----- Original Message ----- From: "OvermindDL1"<overminddl1@gmail.com> To:<boost-users@lists.boost.org>;<boost@lists.boost.org>;<boost-announce@lists.boost.org> Sent: Friday, May 07, 2010 5:36 AM Subject: [boost] [Review] Formal Review: Boost.Move

...

Greetings Boost Developers and Users,

It's my pleasure to announce that the review of Ion Gaztañagas' Move library starts May 10th and lasts until May 24th, 2010, unless an extension occurs.

Hi Ion, I have some questions and comments before writting a review.

Vicente, I think I can give an answer to some of your questions, only so that you can receive a speedier response. Of course, Ion is the final authority.

* The use of the is_movable metafunction is not clear in the documentation. The name let think to a concept check, that is, is T a model of the Movable concept? But the check is related to whether T is convertible to rv<T>&, and it seems you uses it in Boost.Container to avoid conflicts using enable_if/disable_if. Could you clarify this? Should is_movable be renamed to a more specific name? [...] * What returns is_movable if BOOST_ENABLE_MOVE_EMULATION is not defined for compilers with rvalue references?

In C++0x, is_movable detects whether a particular nested typedef exists, a typedef defined automatically by the move enabling macros. However, as I've mentioned to Ion previously, it looks like is_movable is only used in a necessary context within C++03 to detect whether move *emulation* is enabled for a type, so it is currently my belief that is_movable should be defined *only* for C++03. I personally find is_movable to be a perfectly adequate name for this trait, but perhaps its purpose should be more precisely nailed down and explained in the documentation.

* Should the use of BOOST_RV_REF in template classes be protected with is_movable?

I don't think this is necessary as far as using BOOST_RV_REF( T ) as a function parameter type. In C++03, that function will just simply never be instantiated if T isn't movable (because a variable of type BOOST_RV_REF( T ) would never be created).

* Could you add the requirements of a Movable type? Could we have a trait for this?

That is also one of the things I think we should consider.

* I don't know if the next metafunction is correct in C++0x,

// emulation template<typename T> struct rvalue_ref { typedef ::BOOST_MOVE_NAMESPACE::rv< T>& type; };

// C++0x template<typename T> struct rvalue_ref { typedef T&& type; };

If correct, could it be used to replace the macro BOOST_RV_REF?

Because you wouldn't be able to get template parameters to bind to function argument types. Consider template< class T > void f(BOOST_RV_REF( T )); vs template< class T > void f(typename rvalue_ref<T>::type); Other than that, there should be no difference. Indeed, I think I'm going to propose additional metafunctions dealing with rvalue references, e.g., add_rvalue_reference, remove_rvalue_reference, is_rvalue_reference...

* BTW, why BOOST_RV_REF_2_TEMPL_ARGS and BOOST_RV_REF_3_TEMPL_ARGS are needed?

#define BOOST_RV_REF(TYPE)\ ::BOOST_MOVE_NAMESPACE::rv< TYPE>& \ //

#define BOOST_RV_REF_2_TEMPL_ARGS(TYPE, ARG1, ARG2)\ ::BOOST_MOVE_NAMESPACE::rv< TYPE<ARG1, ARG2> >& \ //

#define BOOST_RV_REF_3_TEMPL_ARGS(TYPE, ARG1, ARG2, ARG3)\ ::BOOST_MOVE_NAMESPACE::rv< TYPE<ARG1, ARG2, ARG3> >& \ //

Consider what the preprocessor would make of BOOST_RV_REF( tmpl< T, U > )...oops! The preprocessor thinks you're passing 2 parameters to BOOST_RV_REF :( Ion, instead of BOOST_RV_REF_N_TEMPL_ARGS, I use something called BOOST_RV_REF_TMPL( tmpl, type_seq ) which takes the name of a class template tmpl and a Boost.Preprocessor seq type_seq and expands to an (emulated) rvalue reference to tmpl< type_seq[0], ..., type_seq[n-1] >: #define BOOST_RV_REF_TMPL( tmpl, type_seq ) tmpl < BOOST_PP_SEQ_ENUM( type_seq ) > && or #define BOOST_RV_REF_TMPL( tmpl, type_seq ) ::boost::rv< tmpl < BOOST_PP_SEQ_ENUM( type_seq ) > > & I prefer using these to BOOST_RV_REF_N_TEMPL_ARGS. Something to consider for addition to / replacement of BOOST_RV_REF_N_TEMPL_ARGS.

* has_nothrow_move. What are the expected some performance improvements. of movable classes that specialize this to true? Could an example be added?

I'm hoping more generally to be specific on where we stand with throwing move constructors and, if acceptable, how we intend to support them. If you haven't already, see David Abraham's article at C++Next [1], and the link from there to N2983.

* As the standard uses these more specific ones template<class T> struct has_nothrow_move_constructor;

template<class T> struct has_nothrow_move_assign;

Shouldn't Boost.Move use these

What are the implementations of these? Can you give a reference confirming they are in the (upcoming) standard?

* What about the other move traits? Couldn't we let the user specialize them also?

template<class T> struct has_move_constructor;

template<class T> struct has_move_assign;

template<class T> struct has_trivial_move_constructor;

template<class T> struct has_trivial_move_assign;

I'm unsure whether we should encourage user-specialization for so many metafunctions :/ [...]

Best, _____________________ Vicente Juan Botet Escribá http://viboes.blogspot.com/

- Jeff [1] http://cpp-next.com/archive/2009/10/exceptionally-moving/

Hi

Consider what the preprocessor would make of BOOST_RV_REF( tmpl< T, U > )...oops! The preprocessor thinks you're passing 2 parameters to BOOST_RV_REF :(

What about just using four parenthesis to prevent the comma being interpreted by the preprocessor as a macro argument separator : BOOST_RV_REF((tmpl< T, U >))

----- Original Message ----- From: "Jeffrey Lee Hellrung, Jr." <jhellrung@ucla.edu> To: <boost@lists.boost.org> Sent: Sunday, May 16, 2010 4:10 AM Subject: Re: [boost] [Review] Formal Review: Boost.Move

On 5/15/2010 4:26 PM, vicente.botet wrote:

...

----- Original Message ----- From: "OvermindDL1"<overminddl1@gmail.com> To:<boost-users@lists.boost.org>;<boost@lists.boost.org>;<boost-announce@lists.boost.org> Sent: Friday, May 07, 2010 5:36 AM Subject: [boost] [Review] Formal Review: Boost.Move

Hi Ion, I have some questions and comments before writting a review.

Vicente, I think I can give an answer to some of your questions, only so that you can receive a speedier response. Of course, Ion is the final authority.

Thanks.

...

* The use of the is_movable metafunction is not clear in the documentation. The name let think to a concept check, that is, is T a model of the Movable concept? But the check is related to whether T is convertible to rv<T>&, and it seems you uses it in Boost.Container to avoid conflicts using enable_if/disable_if. Could you clarify this? Should is_movable be renamed to a more specific name? [...] * What returns is_movable if BOOST_ENABLE_MOVE_EMULATION is not defined for compilers with rvalue references?

In C++0x, is_movable detects whether a particular nested typedef exists, a typedef defined automatically by the move enabling macros. However, as I've mentioned to Ion previously, it looks like is_movable is only used in a necessary context within C++03 to detect whether move *emulation* is enabled for a type, so it is currently my belief that is_movable should be defined *only* for C++03. I personally find is_movable to be a perfectly adequate name for this trait, but perhaps its purpose should be more precisely nailed down and explained in the documentation.

Ion if this is the case, could you change the documentation. In C++03, is_movable needs to instantiate rv<T> to detect if T is convertible to T, but for the builting types this should not work as rv<int> generate a compile error. The same applies for enums. Am I missing something?

...

* Should the use of BOOST_RV_REF in template classes be protected with is_movable?

I don't think this is necessary as far as using BOOST_RV_REF( T ) as a function parameter type. In C++03, that function will just simply never be instantiated if T isn't movable (because a variable of type BOOST_RV_REF( T ) would never be created).

You are right for function parameters, but for return types or typedefs we should protect the instantiation, isn"t it?

...

* Could you add the requirements of a Movable type? Could we have a trait for this?

That is also one of the things I think we should consider.

...

* I don't know if the next metafunction is correct in C++0x,

// emulation template<typename T> struct rvalue_ref { typedef ::BOOST_MOVE_NAMESPACE::rv< T>& type; };

// C++0x template<typename T> struct rvalue_ref { typedef T&& type; };

If correct, could it be used to replace the macro BOOST_RV_REF?

Because you wouldn't be able to get template parameters to bind to function argument types. Consider

template< class T > void f(BOOST_RV_REF( T ));

vs

template< class T > void f(typename rvalue_ref<T>::type);

Other than that, there should be no difference.

This avoid to use macro and hides the internal implementation. As builtins don't accept rv instantiation, but copy semantics is equivalent to move semantics

Indeed, I think I'm going to propose additional metafunctions dealing with rvalue references, e.g., add_rvalue_reference, remove_rvalue_reference, is_rvalue_reference...

Yes, this is interesting.

...

* BTW, why BOOST_RV_REF_2_TEMPL_ARGS and BOOST_RV_REF_3_TEMPL_ARGS are needed?

Consider what the preprocessor would make of BOOST_RV_REF( tmpl< T, U > )...oops! The preprocessor thinks you're passing 2 parameters to BOOST_RV_REF :(

Sorry, I have not tried this, but does BOOST_RV_REF(( tmpl< T, U > )) fails?

...

* has_nothrow_move. What are the expected some performance improvements. of movable classes that specialize this to true? Could an example be added?

I'm hoping more generally to be specific on where we stand with throwing move constructors and, if acceptable, how we intend to support them.

If you haven't already, see David Abraham's article at C++Next [1], and the link from there to N2983.

I did, and will do again, but I think that the documentation need to explain explicitly or reference here somthing external.

...

* As the standard uses these more specific ones template<class T> struct has_nothrow_move_constructor;

template<class T> struct has_nothrow_move_assign;

Shouldn't Boost.Move use these

What are the implementations of these?

The same as has_nothrow_move,, false until specialized explicitly to true.

Can you give a reference confirming they are in the (upcoming) standard?

N3092 20.7.2 Header <type_traits> synopsis

...

* What about the other move traits? Couldn't we let the user specialize them also?

template<class T> struct has_move_constructor;

template<class T> struct has_move_assign;

template<class T> struct has_trivial_move_constructor;

template<class T> struct has_trivial_move_assign;

I'm unsure whether we should encourage user-specialization for so many metafunctions :/

Well, this is the current state of C++0x. I will prefer Boost.Move to be close to C++0x. You, or Boost.Move can always define the more general with the specific. Best, Vicente

----- Original Message ----- From: "Mathieu -" <ptr.jetable@gmail.com> To: <boost@lists.boost.org> Sent: Sunday, May 16, 2010 12:46 PM Subject: Re: [boost] [Review] Formal Review: Boost.Move

On 16 May 2010 12:24, vicente.botet <vicente.botet@wanadoo.fr> wrote:

...

...

...

* BTW, why BOOST_RV_REF_2_TEMPL_ARGS and BOOST_RV_REF_3_TEMPL_ARGS are needed?

Consider what the preprocessor would make of BOOST_RV_REF( tmpl< T, U > )...oops! The preprocessor thinks you're passing 2 parameters to BOOST_RV_REF :(

...

Sorry, I have not tried this, but does BOOST_RV_REF(( tmpl< T, U > )) fails?

That's what I tried ( unfortunately AFTER posting :/), but it doesn't work with types because the macro expand to something like :

::boost::rv<(tmpl<T,U>)> which is an invalid template argument..

:( I have no tested it neither before posting. We have then the possibility to write BOOST_RV_REF_2_TEMPL_ARGS(Tmpl, A1, A2) or typename rvalue_ref<Tmpl<A1,A2> >::type I prefer to write code that doesn't use macros as far as it is possible. Best, Vicente

----- Original Message ----- From: "joel falcou" <joel.falcou@lri.fr> To: <boost@lists.boost.org> Sent: Sunday, May 16, 2010 5:45 PM Subject: Re: [boost] [Review] Formal Review: Boost.Move

vicente.botet wrote:

...

:( I have no tested it neither before posting. We have then the possibility to write

BOOST_RV_REF_2_TEMPL_ARGS(Tmpl, A1, A2)

or

typename rvalue_ref<Tmpl<A1,A2> >::type

I prefer to write code that doesn't use macros as far as it is possible.

As havign being bitten by this a lot of time, I found convenient to have specific macro for tempalte with N parameters and one for non-template type.

Do we realy need a macros or a metafunction such as rvalue_ref can be used. Best, Vicente

On 16/05/2010 1:26, vicente.botet wrote:

* The use of the is_movable metafunction is not clear in the documentation. The name let think to a concept check, that is, is T a model of the Movable concept? But the check is related to whether T is convertible to rv<T>&, and it seems you uses it in Boost.Container to avoid conflicts using enable_if/disable_if. Could you clarify this? Should is_movable be renamed to a more specific name?

Is not clear and after discussing it in previous mails, I think it should be present only for C++03 with another name. It detects whether a class implements move emulation functions. It's useful to sfinae out some overloads in generic functions (eg. push_back in vector, since vector<int> should not instantiate at all any rv<int> type).

* Should the use of BOOST_RV_REF in template classes be protected with is_movable?

* What returns is_movable if BOOST_ENABLE_MOVE_EMULATION is not defined for compilers with rvalue references?

I don't have anything in addition to Jeffrey's explantions for those. Thanks Jeffrey!

* Could you add the requirements of a Movable type? Could we have a trait for this?

Yes, please see my previous post replying Jeffrey.

* I don't know if the next metafunction is correct in C++0x, If correct, could it be used to replace the macro BOOST_RV_REF?

See Jeffrey's explanation. Adding more metafunctions would be correct if we find they might be useful for generic programming. But I don't know if rv<T> & is the correct answer for generic programming using C++0x (add_rvalue_reference, remove_rvalue_reference, is_rvalue_reference). We still don't have enough experience with both rv references and this emulation to know the answer.

* BTW, why BOOST_RV_REF_2_TEMPL_ARGS and BOOST_RV_REF_3_TEMPL_ARGS are needed?

Preprocessor limitations, extra parens won't fix ;-) Regarding the use of Boost.Preprocessor, I aim was to write Boost.Move with very few dependencies. Although support for Boost.MPL was requested in previous discussions, I would like to get this without header dependencies, but I guess Boost.TypeTraits will also depend on MPL, although that could be fixed because traits might go to TypeTraits.

* has_nothrow_move. What are the expected some performance improvements. of movable classes that specialize this to true? Could an example be added?

See http://permalink.gmane.org/gmane.comp.lib.boost.devel/204112

* As the standard uses these more specific ones template<class T> struct has_nothrow_move_constructor;

template<class T> struct has_nothrow_move_assign;

Shouldn't Boost.Move use these

Yes, at least if they are stable in the draft. And maybe they should en end in Boost.TypeTraits, along with has_trivial_destructor after move, which can be used to avoid calling destructors in containers when an element is moved and its destructor does not need to release any resource.

Implementation * I would like to know the reason to don't use is_convertible from Boost.TypeTraits and define your own. * The same for identity?

Dependency issues, but I have no problem to change this. Best, Ion

On 18/05/2010 17:22, vicente.botet wrote:

...

Is not clear and after discussing it in previous mails, I think it should be present only for C++03 with another name. It detects whether a class implements move emulation functions. It's useful to sfinae out some overloads in generic functions (eg. push_back in vector, since vector<int> should not instantiate at all any rv<int> type).

What about is_convertible_to_rvale?

Could be, but so many things are convertible to an rvalue...

Do you plan to add some examples that use this metafunction on the documentation? Maybe moving some examples from Boost.Container?

I should, because it has real-world use cases.

My question was because the documentation doesn't state clearly the Jeffrey's explantions. If you will remove this metafunction for C++0x compilers, the issue is no more one.

Ok. I'll remove it.

Doesn't the metafunction solve these issues?

How?

...

Yes, at least if they are stable in the draft. And maybe they should en end in Boost.TypeTraits, along with has_trivial_destructor after move, which can be used to avoid calling destructors in containers when an element is moved and its destructor does not need to release any resource.

Including these on Boost.Move could allows to experiment and give an input to the standard.

Yes, but if move them from Boost.Move to Boost.TypeTraits we can break some code. So Boost.Move should maintain some backwards compatibility, at least, for some time. Best, Ion

OvermindDL1 wrote:

We are approaching the final days of the review and there have been no definitive reviews as of yet, and a lot of talk about design and what to include or not include. If this continues without any actual reviews and none in sight then I will have to end the review period, else if we do start to get some reviews then we can issue an extension if the Review Wizards agree.

I will try to send in a review till Tuesday. Regards, Thomas

On 22/05/2010 13:41, vicente.botet wrote:

Hi Ion,

It seems to me that the optimization mode is more related to a specific class than the complete application. How can interact two parts of the same application that uses different optimization mode? Shouldn't be better to define separated macros?

They could coexist, but generic algorithms could not take advantage of optimized mode advantages, unless they detect this optimized mode and apply a more tailored algorithm for them, which seems absolutely non-generic ;-) I would prefer just having one mode, which is equivalent of having a well-specified MoveConstructible or MoveAssignable properties for any type. But, of course, this is an opinion and reviewers have the last word. Best, Ion

On 22/05/2010 17:00, vicente.botet wrote:

* The class rv should be public and documented, so people prefering to don't use macros can work with the library.

No problem if that's what users prefer.

* As pointed in another post is_movable should be renamed and its intent clarified (and maybe constrained to C++98 mode) with some examples, maybe show some parts of the implementation of a movable container.

Yes, is_movable is useful but the name is not very useful, even less according to the C++0x "movable" concept which includes copy constructible types, that is, anything that can be constructed from a rvalue.

* has_nothrow_move. Clarify on the documentation the expected performance improvements of movable classes that specialize this to true, and add an example on the tutorial.

Ok.

* I think that, as the standard, the library should work with more specific traits than has_nothrow_move, as each operation could have its own specificities. template<class T> struct has_nothrow_move_constructor; template<class T> struct has_nothrow_move_assign;

Ok. By default, I guess those should be false, unless we have compiler intrinsics around.

* I guess the standard traits can not be defined other than by the compiler, but if no the traits could be specialized by the user, the library will provide just the prototype. template<class T> struct has_move_constructor; template<class T> struct has_move_assign; template<class T> struct has_trivial_move_constructor; template<class T> struct has_trivial_move_assign;

Yes, this could be an option. For c++0x we have "noexcept()".

The library can add these on the TypeTraits if this is the prefered place.

This is upt o TypeTraits maintainers. We can start experimenting in Boost.Move, but if in the future we move them to TypeTraits (which seems to be the correct place, we will need to maintain compatibility with those). Maybe move_type_traits.hpp could be the header containing those and in the future this could just include apropriate TypeTraits headers.

* I will add a file boost/move.hpp that include the file boost/move/move.hpp

Ok.

* Why the macros BOOST_MOVABLE_BUT_NOT_COPYABLE and BOOST_COPYABLE_AND_MOVABLE must be included in the private part. For me this is part of the public interface.

I have no preference for this, it was originally in the public part but in previous pre-review comments some preferred in the private part since considered it an "implementation detail".

...

- What is your evaluation of the implementation?

* The implementation is complex as emmulating this language feature is not simple. It will be great if the section "How it works?" explained how forward works on emmulated mode.

Ok.

* I don't like the use of macros to handle with issues Interprocess porting. Guess this is temporary until Interprocess will migrate to use Boost.Move (Or is there a particular issue?). * Implementation details as metafunctions identity and is_convertible should be reused from mpl and type-traits instead of redefining them.

Yes, it's temporary, it was just to maintain a single source for two libraries, because maitenance was becoming a pain. no problem for identity and is_convertible.

...

- What is your evaluation of the documentation?

* I find the index not too much structured. I would put together all the tutorials in a Tutorial section.

What do you mean by "tutorial"?

* I have not found any mention that this is a header only library, maybe I have missed this point.

Ok.

* I would apreciate if the author states explicitly the dependencies to other Boost libraries.

Ok.

* An example showing how to make a container or boost::function movable will help to understand a lot of uses.

Ok. A container would be easier for me.

* The reference section should include a complete detail of the description of each function, its effects, returned values possible exceptions, ... Currently there are too much functions for which the prototype is the single documentation.

Ok.

* I would like that the macros reference description states clearly what is behind the scenes, as the user needs to clearly understand what operations are defined and which one s/he doesn't needs to define.

Ok

* A comparaison with the wrapper way should be welcome as well as a brief history of when move semantics was implemented the first time and how this has evolved over time.

Ok, I consider this somewhat secondary, but I will definitely add it once other suggestions are added to the library

* A Bibliography section compiling all the referred documents should be nice to have.

Ok.

* Duplicated line in two_emulation_modes.html#move.two_emulation_modes.optimized_mode copyable_and_movable cm;

Ok

...

- What is your evaluation of the potential usefulness of the library? - Did you try to use the library? With what compiler? Did you have any problems?

Yes, cygwin gcc-3.4, mingw-4.4 msvc Express9

I have run the tests on Boost.Move and Boost.Containers (C++98 mode) with no problem with cygwin gcc-3.4, mingw-4.4 and msvc Express9.

Just some really minor warning cygwin gcc-3.4 move_iterator.cpp:104:3: warning: no newline at end of file vector_test.cpp:174:3: warning: no newline at end of file

Ok

Bravo!

I have some run-time errors with mingw-4.4, but I suspect that my installation is not as stable as I would.

I think this is a strict aliasing issue, arised in Interprocess, because we are accesing T from a type ::boost::rv<T> that does not exist. Adding a alias attribute seems to fix the problem. See: https://svn.boost.org/trac/boost/ticket/3950

Ion, thanks again for all the work done. _________________

Thanks for the review Ion

AMDG Terry Golubiewski wrote:

I agree that boost::rv<> should not be in the move_detail namespace. I found using the macros to be annoying and did not use them. I did add...

typedef boost::rv<T>& rv_ref;

... to my movable classes for convenience. I never felt a need for const_rv_ref though.

The main reason for using the macros is to get an automatic upgrade to real rvalue references when they are available. In Christ, Steven Watanabe

I think for boost developers the use of macros is good. For my "user" code. I would rather use boost::rv<T>& or just T::rv_ref. Then (after a few years, unfortunately) I can grep for \<rv\> and replace them with &&. I didn't mean that the macros should be removed, just that I would like the option of not using them. Since rv<T>& represents a standard language construct, and it is already in namespace boost, I don't think it needs to be "hidden" in move_detail, which suggests that it should not be used by user-code. terry ----- Original Message ----- From: "Steven Watanabe" <watanabesj@gmail.com> Newsgroups: gmane.comp.lib.boost.devel To: <boost@lists.boost.org> Sent: Saturday, May 22, 2010 6:17 PM Subject: Re: [Review] Formal Review: Boost.Move

AMDG

Terry Golubiewski wrote:

...

I agree that boost::rv<> should not be in the move_detail namespace. I found using the macros to be annoying and did not use them. I did add...

typedef boost::rv<T>& rv_ref;

... to my movable classes for convenience. I never felt a need for const_rv_ref though.

The main reason for using the macros is to get an automatic upgrade to real rvalue references when they are available.

In Christ, Steven Watanabe

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

----- Original Message ----- From: "Jeffrey Lee Hellrung, Jr." <jhellrung@ucla.edu> Newsgroups: gmane.comp.lib.boost.devel To: <boost@lists.boost.org> Sent: Saturday, May 22, 2010 4:21 PM Subject: Re: [Review] Formal Review: Boost.Move

On 5/22/2010 1:14 PM, Terry Golubiewski wrote:

...

* class rv: I needed the destructor to be public to compile for Microsoft VC2008.

I never had any problems with the current definition of boost::rv on MSVC9. No instances of rv<T> should ever be created, only references. Do you remember the context where you needed rv<T> to have a public destructor???

No, but I defintely needed it at some point. I think it had to do with my hacks on is_convertible<>. When I get some time tomorrow, I'll make ~rv() private again and see what breaks. terry

On 25/05/2010 13:27, Terry Golubiewski wrote:

In my post, I had suggested alternate implementation of forward() and I thought I needed another version of move(). Also, should boost have versions of add_rv_reference<> and such? What do you think?

forward is a version that is thought to be a correct implementation for implementation #5 from N2951 (code was provided by a LWG member). I'll need to check what move does in C++0x with const references. Regarding add_rv_reference<>, I don't know if that's useful for Boost.Move, it should be in TypeTraits, no doubt, but I don't know if use-cases for add_rv_reference will allow using emulation mode. We'll see this when boosters start pushing move semantics to the limit. Best, Ion

The review is over, I shall still be looking at newly posted reviews and reports over the next day or so as I get time to combine all the data. So this is last call!

I didn't have enough time for an official review (and will not have coming days), so herewith I express my interest in this library. Thanks for writing it and for managing the review. Regards, Barend

On 26/05/2010 13:46, vicente.botet wrote:

Hi,

The following doesn't compiles. void fct(BOOST_RVALUE_REF(movable_only) mo, int x){...} ... movable_only m; boost::function<void()> f_one = boost::bind(&fct, boost::move(m), 1);

Im not sure if the following is correct, boost::function<void()> f_one = boost::bind(&fct, boost::ref(boost::move(m)), 1);

What is the correct way to use move semantics with Boost.Bind? Do we need some Boost.Bind adaptations?

I don't know, maybe Boost.Bind should catch rvalues differently, but like with perfect forwarding (after all, bind is about forwarding) we'll need some limitations or tweaks. Maybe we need some ::boost::rv_ref()/boost::rv_reference_wrapper in Boost.Ref to pass rvalues to bind. And them Bind could forward them to the target call. Bind documentation says: "By default, bind makes a copy of the provided function object. boost::ref and boost::cref can be used to make it store a reference to the function object, rather than a copy. This can be useful when the function object is noncopyable, expensive to copy, or contains state; of course, in this case the programmer is expected to ensure that the function object is not destroyed while it's still being used." So evidently we can't pass movable-only objects unless we have some rvalue reference wrapper. Best, Ion

On 26/05/2010 16:13, vicente.botet wrote:

Can ::boost::rv_ref()/boost::rv_reference_wrapper be implemented without using boost::rv?

Yes, they can use BOOST_RV_REF + move().

If not, is this one reason to make it public and documented?

Yes and no, it depends on what Boost.Ref authors prefer ;-) I'll try to experiment with this when I have time so that we can make a concrete proposal for Boost.Ref. Best, Ion