Subject: Re: [boost] [Review] Formal Review: Boost.Move
From: Terry Golubiewski (tjgolubi_at_[hidden])
Date: 2010-05-22 17:18:10
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.
----- Original Message -----
From: "vicente.botet" <vicente.botet_at_[hidden]>
Sent: Saturday, May 22, 2010 10:00 AM
Subject: Re: [Review] Formal Review: Boost.Move
> ----- Original Message -----
> From: "OvermindDL1" <overminddl1_at_[hidden]>
> To: <boost-users_at_[hidden]>; <boost_at_[hidden]>;
> Sent: Friday, May 07, 2010 5:36 AM
> Subject: [boost] [Review] Formal Review: Boost.Move
>> 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.
> I see that there is not too much participation on the review of this
> library. Either people is not interested on Move semantics (I doubt),
> either they are interested but they have an implementation provided by
> some compilers in C++0x. This library will allow everybody to play with
> move semantics without waiting you can use a C++0x compiler in your work,
> and way is better improving the performances of your applications.
> Anyway, here it is my review.
> I was waiting for this library since a too long time. Most people think
> that this is an emulation library, but it is more than that as it
> implements most of the C++0x Move semantics classes and functions. In
> addition with the use of the proposed macros the user can write almost
> portable applications that can rin on c++98 and C++0x compilers.
> The fact that Ion has already ported its libraries Container and Intrusive
> libraries as well as the unordered library let me think that there will be
> not too much holes. It will be great to have also Interprocess (completly
> removing the need of the Interprocess specific macors) and Thread
> libraries ported :)
> How can Boost.Functional/Forward and Boost.Move interact?
>> - What is your evaluation of the design?
> This is a little library covering a complex domain. We have no found a
> better way to emulate move semantics. The limitations are clearly
> identified and don't add constraints to the common uses. So yes, this is a
> good design.
> * The class rv should be public and documented, so people prefering to
> don't use macros can work with the library.
> * 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.
> * 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.
> * 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;
> * 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;
> The library can add these on the TypeTraits if this is the prefered place.
> * I will add a file boost/move.hpp that include the file
> * How can interact two parts of the same application that uses different
> optimization mode? Shouldn't be better to define separated macros? Am I
> missing something evident?
> * Why the macros BOOST_MOVABLE_BUT_NOT_COPYABLE and
> must be included in the private part. For me this is part of the public
>> - 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.
> * 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.
>> - What is your evaluation of the documentation?
> * The tutorial is well written.
> * I find the index not too much structured. I would put together all the
> tutorials in a Tutorial section.
> * I have not found any mention that this is a header only library, maybe I
> have missed this point.
> * I would apreciate if the author states explicitly the dependencies to
> other Boost libraries.
> * An example showing how to make a container or boost::function movable
> will help to understand a lot of uses.
> I will move Containers and move semantics to an Examples section
> * 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.
> * 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.
> * 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.
> * A Bibliography section compiling all the referred documents should be
> nice to have.
> * Duplicated line in
> copyable_and_movable cm;
>> - 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
> 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
> I have some run-time errors with mingw-4.4, but I suspect that my
> installation is not as stable as I would.
>> - How much effort did you put into your evaluation? A glance? A quick
>> - reading? In-depth study?
> A long walk.
>> - Are you knowledgeable about the problem domain?
> I understand the domain, but unfortunately I'm not an expert in.
>> - Do you think the library should be accepted as a Boost library?
> Yes. Boost must include a common library emmulating move semantics even if
> there are some known limitations.
> Ion, thanks again for all the work done.
> Vicente Juan Botet EscribÃ¡
> Unsubscribe & other changes:
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk