Subject: Re: [boost] [Review] Formal Review: Boost.Move
From: Terry Golubiewski (tjgolubi_at_[hidden])
Date: 2010-05-25 07:27:39
In my post, I had suggested alternate implementation of forward()
and I thought I needed another version of move().
Also, should boost have versions of add_rv_reference<> and such?
What do you think?
terry
----- Original Message -----
From: "Terry Golubiewski" <tjgolubi_at_[hidden]>
Newsgroups: gmane.comp.lib.boost.devel
To: <boost_at_[hidden]>
Sent: Saturday, May 22, 2010 3:14 PM
Subject: Re: [Review] Formal Review: Boost.Move
>> - What is your evaluation of the design?
>
> The design is great, bringing the best of several good designs together
> and
> supports C++2003 and C++0X.
>
>> - What is your evaluation of the implementation? Very good.
>> - What is your evaluation of the documentation? Didn't read it (yet).
>> - What is your evaluation of the potential usefulness of the library?
>
> This library is long overdue!
> I have been using move emulation for several years. There are several
> things that you just can't do well without move semantics.
> Also, boost has already several different methods of move emulation that
> should be united using this library.
> Move semantics do significantly improve program performance without much
> extra programmer effort, especially from users.
> I only use a modified version of move.hpp and haven't used any of the
> move-aware containers yet.
>
>> - Did you try to use the library? With what compiler? Did you have any
>> problems?
>
> The modified move.hpp that I use is attached.
> I have been using a revised (see below and attached) version of this
> library
> for several months.
> I have implemented a version of Howard Hinnants unique_ptr<> emulation
> using
> it.
> I have successfully run Howard Hinnant's unique_ptr<> test suite using it.
> I use move() for message passing, resource management (ala Alexandrescu's
> LockingPtr and InternallyLocked), and C++0x thread emulation.
> I don't have/use move-aware containers yet.
> I had a few problems given in my notes below.
>
>> - How much effort did you put into your evaluation? A glance? A quick
>> - reading? In-depth study?
>
> In-depth study of move.hpp
>
>> - Are you knowledgeable about the problem domain?
>
> Yes.
>
>> - Do you think the library should be accepted as a Boost library?
>
> Yes!!! (emphatically)
>
> * Having extra '{' in "//namespace *{" comments makes my editor (vim)
> unhappy.
>
> * class rv: I needed the destructor to be public to compile
> for Microsoft VC2008.
>
> * for Gnu C++, I added __attribute__((__may_alias__)) to rv<>.
>
> * I needed a slightly different version of is_convertible for some reason
> that
> I've forgotton now. This version was mostly taken from Howard Hinnant's
> unique_ptr<> emulation. Then I hacked it until it worked for Microsoft
> Visual C++ 2008 and GCC 4.3.
>
> namespace move_detail {
>
> namespace is_conv_impl {
>
> typedef char one;
> struct two { char _[2]; };
> template<typename T> one test1(const T&);
> template<typename T> two test1(...);
> template<typename T> one test2(T);
> template<typename T> two test2(...);
> template<typename T> T source();
>
> } // is_conv_impl
>
> template<class Fm, class To, bool = true>
> struct is_convertible_helper: public boost::is_convertible<Fm, To> { };
>
> template<class Fm, class To>
> struct is_convertible_test1: public boost::mpl::bool_<
> sizeof(is_conv_impl::test1<To>(is_conv_impl::source<Fm>())) == 1>
> { };
>
> template<class Fm, class To>
> struct is_convertible_test2: public boost::mpl::bool_<
> sizeof(is_conv_impl::test2<To>(is_conv_impl::source<Fm>())) == 1>
> { };
>
> #if defined(__GNUG__)
> template<class Fm, class To>
> struct is_convertible_helper<Fm, To, true>
> : public is_convertible_test1<Fm, To> { };
>
> // For move-only types
> template<class T>
> struct is_convertible_helper<T, T, true>
> : public is_convertible_test2<T, T> { };
>
> #elif defined(_MSCVER)
>
> template <class Fm, class To>
> struct is_convertible_helper<Fm, To, true>
> : public is_convertible_test2<Fm, To> { };
>
> #endif
>
> template<class Fm, class To>
> struct is_convertible: public is_convertible_helper<Fm, To> { };
>
> } // move_detail
>
> * Added is_class<T> check to is_movable
>
> template<class T>
> struct is_movable
> : public boost::mpl::and_< boost::is_class<T>
> , move_detail::is_convertible< T, rv<T>& >
> >
> { };
>
> * Yet another boost::identity<T>?
>
> * Add support for:
>
> is_rvalue_reference<>
> is_lvalue_reference<>
> remove_reference<>
> etc.
>
> * Add "inline" to move()'s and forward()'s.
>
> * Add additional move():
>
> template<class T> inline
> typename boost::disable_if< is_movable<T>, const T& >::type
> move(const T& x) { return x; }
>
> * My forward()'s are different. I remember changing them because of a
> change
> * in the C++0X standard.
>
> template<class T, class U> inline
> typename boost::enable_if_c<
> is_lvalue_reference<T>::value
> && move_detail::is_convertible< U*
> , typename remove_reference<T>::type*
> >::value
> , T >::type
> forward(U& u) { return static_cast<T>(u); }
>
> template<class T, class U> inline
> typename boost::enable_if_c<
> !is_reference<T>::value
> && move_detail::is_convertible< U*, T* >::value
> , typename add_rvalue_reference<T>::type >::type
> forward(U& u) { return move(u); }
>
> template<class T, class U> inline
> typename boost::enable_if_c<
> !is_reference<T>::value
> && move_detail::is_convertible< U* , T* >::value
> , const T& >::type
> forward(const U& u) { return static_cast<const T&>(u); }
>
> template<class T, class U> inline
> typename boost::enable_if_c<
> !is_reference<T>::value
> && move_detail::is_convertible< U* , T* >::value
> , typename add_rvalue_reference<T>::type >::type
> forward(rv<U>& u) { return move(static_cast<T&>(static_cast<U&>(u))); }
>
> * I have a swap defined like this. Perhaps boost::swap should be updated
> if
> * the move library is accepted.
>
> template<class T1, class T2> inline
> void swap(T1& a, T2& b) {
> T1 tmp(std0x::move(a));
> a = std0x::move(b);
> b = std0x::move(tmp);
> } // swap
>
> * In class move_iterator:
> operator++(int),
> operator--(int),
> operator+(difference_type n) const
> operator-(difference_type n) const
> have extra <iterator_type>.
>
> * Added "inline" to make_move_iterator().
> make_move_iterator()
> uninitialized_move()
> uninitialized_move_move_iterator()
> uninitialized_copy_or_move()
>
--------------------------------------------------------------------------------
> _______________________________________________
> Unsubscribe & other changes:
> http://listarchives.boost.org/mailman/listinfo.cgi/boost