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

17 Jul 2012


      On Tue, Jul 17, 2012 at 2:06 AM, Krzysztof Czainski <1czajnik@gmail.com>wrote:
...
2012/7/17 Jeffrey Lee Hellrung, Jr. 
...
[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
...
::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
...
::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, 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, 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
...
::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