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

Subject: Re: [boost] [Review] Formal Review: Boost.Move
From: vicente.botet (vicente.botet_at_[hidden])
Date: 2010-05-15 19:26:01

----- 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

> 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.

* 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?

* 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?

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

* 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?

* 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> >& \
//

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

* 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 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;

* It would be great to see what is specific to the Boost.Move that doesn't appear in C++0x; for example is_movable, has_nothrow_move.

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?

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