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

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://lists.boost.org/mailman/listinfo.cgi/boost