Re: [Boost-users] [move] case study: simple cloning smart pointer

Subject: Re: [Boost-users] [move] case study: simple cloning smart pointer
From: Jeffrey Lee Hellrung, Jr. (jeffrey.hellrung_at_[hidden])
Date: 2012-07-17 10:25:26

On Tue, Jul 17, 2012 at 2:06 AM, Krzysztof Czainski <1czajnik_at_[hidden]>wrote:

> 2012/7/17 Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung_at_[hidden]>
>
>> [Just a few suggestions...]
>>
> [snip]
>
>> This is where the hard part comes in - the conversion constructors and
>>> assignment operators. First version looks like this:
>>>
>>> {{{
>>> private:
>>>
>>> #if !defined(BY_VALUE)
>>>
>>>     BOOST_COPYABLE_AND_MOVABLE(ptr)
>>>
>>> public:
>>>
>>>     // generalized copy ctor for pointers to derived
>>>      template < class U >
>>>     ptr( ptr<U> const& b,
>>>             typename boost::enable_if< boost::is_convertible<U*,T*>
>>> >::type* = 0 )
>>>     : p_( clone_me(b.get()) ) { ++ptr_s; std::cout << "Cp "; }
>>>
>>>     // generalized move ctor for pointers to derived
>>>     template < class U >
>>>     ptr( BOOST_RV_REF(ptr<U>) b,
>>>             typename boost::enable_if< boost::is_convertible<U*,T*>
>>> >::type* = 0 )
>>>     : p_( b.release() ) { ++ptr_s; std::cout << "Mp "; }
>>>
>>
>> I think you'll generally do better with the conversion copy constructor
>> taking by-value and relying on RVO (in C++03). (Implicit) Rvalues of ptr<U>
>> won't be moved into the newly constructed ptr<T> as written above. Note
>> that the RV_REF overload is still useful even if you change the conversion
>> copy constructor to taking by-value. Of course, in C++11, you can't have
>> both by-value and rvalue reference overloads :(
>>
>>     ptr& operator=( BOOST_COPY_ASSIGN_REF(ptr) b )
>>>     {
>>>         T* tmp = clone_me(b.get()); // this can throw
>>>         boost::checked_delete(p_);
>>>         p_ = tmp;
>>>         return *this;
>>>     }
>>>
>>>     ptr& operator=( BOOST_RV_REF(ptr) b )
>>>     {
>>>         boost::checked_delete(p_);
>>>         p_ = b.release();
>>>         return *this;
>>>     }
>>>
>>>     template < class U >
>>>     typename boost::enable_if< boost::is_convertible<U*,T*>, ptr& >::type
>>>     operator=( ptr<U> const& b )
>>>     {
>>>         T* tmp = clone_me(b.get()); // this could throw
>>>         boost::checked_delete(p_);
>>>         p_ = tmp;
>>>         return *this;
>>>     }
>>>
>>
>> Probably want to disable this overload when U == T. And, again, by-value
>> is probably what you want in C++03.
>>
>
> Oh yeah, right, I will add that condition. Thanks.
>
>     template < class U >
>>>     typename boost::enable_if< boost::is_convertible<U*,T*>, ptr& >::type
>>>     operator=( BOOST_RV_REF(ptr<U>) b )
>>>     {
>>>         boost::checked_delete(p_);
>>>         p_ = b.release();
>>>         return *this;
>>>     }
>>> }}}
>>> This was long, and contains some tricky places, like the one commented
>>> 'this could throw'. Did I even get it right? I think so, but I'm just
>>> beginning to learn about move semantics here ;-)
>>>
>>
>> I admit, I didn't really look at the bodies, so no comments regarding
>> correctness there :)
>>
> [snip]
>
>> And now for the costs. I compiled the attached code with MinGW-4.5.0 in 4
>>> configurations: -std=c++0x disabled/enabled, and version 1/2. I compared
>>> the program output to derive the conclusions:
>>>
>>> C++03: version 1 vs. version 2:
>>> - whenever a conversion is needed (about half of the use cases above),
>>> version 2 introduces an additional temporary ptr<> object (without a deep
>>> copy); in one case it's 2 additional temporary ptr<>s;
>>> - the last 4 use cases {{{ ptr<A> b = make_b(); ptr<A> c( make_b() ); a
>>> = make_a(); b = make_b(); }}} introduce a deep copy in version 1, while
>>> while the deep copy is avoided in version 2.
>>>
>>
>> I'm pretty sure these extraneous deep copies would be avoided if you make
>> the changes I suggested above.
>>
>
> You are right, and the second version (when BY_VALUE is defined) is my
> attempt at solving that as you suggest. Please, could you have a look
> again, and let me know if this is what you have in mind:
>

I think the only downside is assignment constructs and destructs an extra
object more than necessary. When move construction and swapping amounts to
just a couple of pointer assignments, it's probably not a big deal, but it
can start to make a different once your class has more members to operate
on.

>
>> Anyway, next comes the second version: implementing conversion
>> constructors and assignment operators in terms of pass-by-value and swap.
>> Note the use of BOOST_COPYABLE_AND_MOVABLE_ALT macro so that an
>> operator=(ptr&) isn't inserted.
>> {{{
>> #else // BY_VALUE
>>
>>     BOOST_COPYABLE_AND_MOVABLE_ALT(ptr)
>>
>> public:
>>
>>     // generalized copy/move constructor implemented by pass-by-value &
>> steal
>>     template < class U >
>>     ptr( ptr<U> b,
>>             typename boost::enable_if< boost::is_convertible<U*,T*>
>> >::type* = 0 )
>>     : p_( b.release() ) { ++ptr_s; std::cout << "Vp "; }
>>
>>     // assignment - works for all types convertible to ptr
>>     ptr& operator=( ptr b )
>>     {
>>         swap(*this,b);
>>         return *this;
>>     }
>>
>> #endif // BY_VALUE
>> }}}
>> Compared to the first version, this is really simple. I see no tricky
>> parts here.
>>
>> [snip]
>
>> Hope that provides with additional insight,
>> - Jeff
>>
>
> Thank you for your reply, Jeff.
> Kris
>