On 5/10/2010 3:24 PM, OvermindDL1 wrote:
Greetings Boost Developers and Users,
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. [...]
I believe Ion's Move library should be accepted in some form or another. I'm obviously hoping all of the following issues can be addressed, but I definitely understand if they cannot be right away (priorities...) or are flat-out rejected ;) I think this library is extremely important, at least in the short-to-middle-term (until rvalue references are sufficiently prevalent). There is a lot of ingenuity in making the move emulation work, and Ion has made it very accessible and managed to make differences between C++03 and C++0x vanish behind macros and free functions. This is great, and I only wish to point out some areas where I think things can be improved. I suggest the following changes should be effected prior to release. If you want elaboration, feel free to ask or reference the archives. -------- - Document the existence of boost::rv<T> and that boost::rv<T>& represents an emulated rvalue reference in C++03. Define the requirements a class must satisfy to interoperate with the move machinery, especially in C++03. I don't know what these should be, precisely, or if they necessarily need to align with the standard concepts of Movable/MoveConstructible/MoveAssignable/whatever, but I did give an idea of what such a requirement might look like in a previous email. I'm willing to bat ideas around regarding this. - Relegate is_movable to only be used in C++03, and document its use as such. I'm fine with the name "is_movable", although I understand it isn't in line with the standard's concept of movable, so I would understand a name change...has_move_emulation_enabled? ;) - I remember why I needed an is_class within the def'n of is_movable: so that is_movable< X& > evaluated to false for class types X (instead of giving a compiler error). Thus, I think the def'n of is_movable should be something like template< class T > struct is_movable : mpl::and_< is_class<T>, mpl::not_< is_rvalue_reference<T> >, is_convertible< T, rv<T>& > > { }; - Either a) provide 2 separate macros to enable move emulation, one "safe"/"friendly" and the other "optimal"; or b) provide a single macro to enable "safe"/"friendly" (i.e., non-optimal) move emulation. I think, if we're going to package 2 possible emulation models, we should make this as local an option as possible, not as global as possible. And given the optimal move emulation model's interaction with legacy code (e.g., std::pair), I think the "safe"/"friendly" model should be encouraged. - Something definitive on throwing move constructors. I feel strangely unsatisfied with this so far. What about Vicente's proposal to use has_nothrow_move_constructor and has_nothrow_move_assign rather than has_nothrow_move? What about move_if_noexcept from N2983? Does boost have support for the noexcept keyword? Failing compiler detection of non-throwing move constructors/assignment operators, I think it would be a good idea for the move emulation macros to tag classes as non-throwing move constructible / non-throwing move assignable, and have the corresponding trait, by default, detect this tag. I know this would have problems for recursive containers, but it seems like this is a special case, there is a workaround in such cases (partial specialization), and it allows users of the emulation macros to automatically take advantage of the performance benefits offered by detecting non-throwing move construction and assignment. [This paragraph is really just turning into a bunch of random thoughts that aren't really organized at all...] -------- I suggest the following additions could be made to improve the library. -------- - Techniques to capture (implicitly created) rvalues with emulated rvalue references in C++03, e.g., for push_back, when you have knowledge of the types of objects you want to capture. I.e., expound on the following technique to capture rvalues of type T, while still allowing implicit conversions to T: 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... } and the following, if you don't need to support conversions to T: void f(T& x) { ...copy x... } void f(rv<T> const& x) { ...copy x... } void f(rv<T>& x) { ...move x... } - TypeTraits metafunctions is_rvalue_reference, add_rvalue_reference, and remove_rvalue_reference. I'm currently using the following definitions: template< class T > struct is_rvalue_reference : boost::false_type { }; #ifndef BOOST_NO_RVALUE_REFERENCES template< class T > struct is_rvalue_reference< T&& > : true_type { }; #else // #ifndef BOOST_NO_RVALUE_REFERENCES template< class T > struct is_rvalue_reference< rv<T>& > : true_type { }; template< class T > struct is_rvalue_reference< const rv<T>& > : true_type { }; #endif // #ifndef BOOST_NO_RVALUE_REFERENCES #ifndef BOOST_NO_RVALUE_REFERENCES template< class T > struct add_rvalue_reference { typedef T&& type; }; #else // #ifndef BOOST_NO_RVALUE_REFERENCES namespace detail_add_rvalue_reference { template< class T, bool = is_movable<T>::value > struct add_rvalue_reference_impl { typedef T type; }; template< class T > struct add_rvalue_reference_impl< T, true > { typedef rv<T>& type; }; } // namespace detail_add_rvalue_reference template< class T > struct add_rvalue_reference : detail_add_rvalue_reference::add_rvalue_reference_impl<T> { }; #endif // #ifndef BOOST_NO_RVALUE_REFERENCES template< class T > struct remove_rvalue_reference { typedef T type; }; #ifndef BOOST_NO_RVALUE_REFERENCES template< class T > struct remove_rvalue_reference< T&& > { typedef T type; }; #else // #ifndef BOOST_NO_RVALUE_REFERENCES template< class T > struct remove_rvalue_reference< rv<T> > { typedef T type; }; template< class T > struct remove_rvalue_reference< const rv<T> > { typedef T type; }; template< class T > struct remove_rvalue_reference< volatile rv<T> > { typedef T type; }; template< class T > struct remove_rvalue_reference< const volatile rv<T>
{ typedef T type; }; template< class T > struct remove_rvalue_reference< rv<T>& > { typedef T type; }; template< class T > struct remove_rvalue_reference< const rv<T>& > { typedef T type; }; template< class T > struct remove_rvalue_reference< volatile rv<T>& > { typedef T type; }; template< class T > struct remove_rvalue_reference< const volatile rv<T>& > { typedef T type; }; #endif // #ifndef BOOST_NO_RVALUE_REFERENCES
- TypeTraits metafunctions is_lvalue_reference, add_lvalue_reference, and remove_lvalue_reference ? Perhaps add_reference and remove_reference can be modified so that they behave wrt emulated rvalue references the same as wrt real rvalue references, i.e., add_reference< rv<T>& > -> T& rather than rv<T>& (since T&& & -> T&). - Add'l TypeTraits has_[trivial_]move_{constructor,assign}...? - An as_lvalue(T& x) function, which amounts to an identity operation in C++0x, but strips emulated rvalue references in C++03. This may be necessary to prevent "accidental moves". - Consider BOOST_RV_REF_TMPL (from a previous email) in addition to / as a replacement for BOOST_RV_REF_N_TEMPL_ARGS. -------- I tried to summarize as much as possible the prior msgs we and others had exchanged, but there may be something I left out. In any case, I think this is sufficiently lengthy the way it is... ;) Let me know what you think, - Jeff