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/