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

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.

terry

----- Original Message ----- 
From: "vicente.botet" <vicente.botet_at_[hidden]>
Newsgroups: gmane.comp.lib.boost.devel
To: <boost_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]>; 
> <boost-announce_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.
>
> Hi,
>
> 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 
> boost/move/move.hpp
>
> * 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 
> BOOST_COPYABLE_AND_MOVABLE
> must be included in the private part. For me this is part of the public 
> interface.
>
>> - 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 
> two_emulation_modes.html#move.two_emulation_modes.optimized_mode
>   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 
>> 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
>
> Bravo!
>
> 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á
> http://viboes.blogspot.com/
>
>
> _______________________________________________
> Unsubscribe & other changes: 
> http://lists.boost.org/mailman/listinfo.cgi/boost