copy_ptr: request for code review.

Discussion in 'C++' started by Kai-Uwe Bux, Aug 22, 2005.

  1. Kai-Uwe Bux

    Kai-Uwe Bux Guest

    Hi folks,


    in another thread there is some discussion about cloning smart pointers. So
    I started thinking about it and stumbled upon a rather simple generic
    implementation. However, it uses dynamic_cast<>, and I am not really
    familiar with that cast. So, I am not sure whether the code is correct.
    Please have a look and let me know how to improve this.



    Sorry for the long post

    Thanks

    Kai-Uwe Bux




    // copy_ptr.cc (C) Kai-Uwe Bux [2005]
    // ==================================

    /*
    | - Upon construction, copy_ptr<T> takes pointee ownership
    | a D* where D is a type derived from T.
    | - In any copy construction or assignment a copy of the
    | pointee is created. There should be no slicing.
    | - Upon destruction the pointee is destroyed.
    | - Intended use is within STL containers.
    */

    // we swap:
    #include <algorithm>

    // The clone function:
    template < typename T, typename D >
    T * clone ( T * ptr ) {
    return( new D ( *( dynamic_cast<D*>( ptr ) ) ) );
    }

    // forward declarations:
    template < typename T >
    class copy_ptr;

    template < typename T >
    void swap ( copy_ptr< T > &, copy_ptr< T > & );


    // implementation:
    template < typename T >
    class copy_ptr {

    friend void swap<> ( copy_ptr<T> &, copy_ptr<T> & );

    /*
    The idea is that in addition to a pointer, we also need
    a pointer to the appropriate clone function.
    */
    T * raw_ptr;
    T * ( *clone_fct ) ( T * );

    public:

    copy_ptr ( void )
    : raw_ptr ( new T )
    , clone_fct( clone<T,T> )
    {}

    template < typename D >
    copy_ptr ( D * ptr )
    : raw_ptr ( ptr )
    , clone_fct ( clone<T,D> )
    {}

    // copy construction clones:
    copy_ptr ( copy_ptr const & other )
    : raw_ptr ( other.clone_fct( other.raw_ptr ) )
    , clone_fct ( other.clone_fct )
    {}

    // destruction frees the pointee
    ~copy_ptr ( void ) {
    delete( raw_ptr );
    }

    // assignment reduces to copy construction:
    copy_ptr & operator= ( copy_ptr const & other ) {
    copy_ptr dummy ( other );
    swap( *this, dummy );
    return( *this );
    }

    T const * operator-> ( void ) const {
    return( raw_ptr );
    }

    T * operator-> ( void ) {
    return( raw_ptr );
    }

    T const & operator* ( void ) const {
    return( *raw_ptr );
    }

    T & operator* ( void ) {
    return( *raw_ptr );
    }

    }; // copy_ptr<T>

    template < typename T >
    void swap ( copy_ptr< T > & p, copy_ptr< T > & q ) {
    std::swap( p.raw_ptr, q.raw_ptr );
    std::swap( p.clone_fct, p.clone_fct );
    }


    // a sanity check:

    #include <iostream>

    struct Base {

    Base ( void ) {
    std::cout << "base is born.\n";
    }

    Base ( Base const & other ) {
    std::cout << "base is copied.\n";
    }

    virtual ~Base () {
    std::cout << "base dies.\n";
    }

    virtual
    std::eek:stream & dump ( std::eek:stream & ostr ) const {
    return( ostr << "base" );
    }

    };

    std::eek:stream & operator<< ( std::eek:stream & ostr,
    Base const & obj ) {
    return( obj.dump( ostr ) );
    }

    struct Derived : public Base {

    Derived ( void ) {
    std::cout << "derived is born.\n";
    }

    Derived ( Derived const & other )
    : Base ( other )
    {
    std::cout << "derived is copied.\n";
    }

    virtual ~Derived () {
    std::cout << "derived dies.\n";
    }

    virtual
    std::eek:stream & dump ( std::eek:stream & ostr ) const {
    return( ostr << "derived" );
    }

    };

    int main ( void ) {
    copy_ptr< Base > a_ptr;
    copy_ptr< Base > b_ptr ( new Derived() );

    std::cout << '\n' << *a_ptr;
    std::cout << " " << *b_ptr << "\n\n";

    a_ptr = b_ptr;
    std::cout << '\n' << *a_ptr;
    std::cout << " " << *b_ptr << "\n\n";

    }
     
    Kai-Uwe Bux, Aug 22, 2005
    #1
    1. Advertising

  2. Kai-Uwe Bux

    Alipha Guest

    Kai-Uwe Bux wrote:
    > Hi folks,
    >
    >
    > in another thread there is some discussion about cloning smart pointers. So
    > I started thinking about it and stumbled upon a rather simple generic
    > implementation. However, it uses dynamic_cast<>, and I am not really
    > familiar with that cast. So, I am not sure whether the code is correct.
    > Please have a look and let me know how to improve this.
    >


    The code appears to be correct and usable, but perhaps can be improved.

    >
    >
    > Sorry for the long post
    >
    > Thanks
    >
    > Kai-Uwe Bux
    >
    >
    >
    >
    > // copy_ptr.cc (C) Kai-Uwe Bux [2005]
    > // ==================================
    >
    > /*
    > | - Upon construction, copy_ptr<T> takes pointee ownership
    > | a D* where D is a type derived from T.
    > | - In any copy construction or assignment a copy of the
    > | pointee is created. There should be no slicing.
    > | - Upon destruction the pointee is destroyed.
    > | - Intended use is within STL containers.
    > */
    >
    > // we swap:
    > #include <algorithm>
    >
    > // The clone function:
    > template < typename T, typename D >
    > T * clone ( T * ptr ) {
    > return( new D ( *( dynamic_cast<D*>( ptr ) ) ) );
    > }


    no check to see if dynamic_cast was successful?

    >
    > // forward declarations:
    > template < typename T >
    > class copy_ptr;
    >
    > template < typename T >
    > void swap ( copy_ptr< T > &, copy_ptr< T > & );
    >
    >
    > // implementation:
    > template < typename T >
    > class copy_ptr {
    >
    > friend void swap<> ( copy_ptr<T> &, copy_ptr<T> & );
    >
    > /*
    > The idea is that in addition to a pointer, we also need
    > a pointer to the appropriate clone function.
    > */
    > T * raw_ptr;
    > T * ( *clone_fct ) ( T * );
    >
    > public:
    >
    > copy_ptr ( void )
    > : raw_ptr ( new T )
    > , clone_fct( clone<T,T> )
    > {}
    >
    > template < typename D >
    > copy_ptr ( D * ptr )
    > : raw_ptr ( ptr )
    > , clone_fct ( clone<T,D> )
    > {}
    >
    > // copy construction clones:
    > copy_ptr ( copy_ptr const & other )
    > : raw_ptr ( other.clone_fct( other.raw_ptr ) )
    > , clone_fct ( other.clone_fct )
    > {}
    >
    > // destruction frees the pointee
    > ~copy_ptr ( void ) {
    > delete( raw_ptr );
    > }
    >
    > // assignment reduces to copy construction:
    > copy_ptr & operator= ( copy_ptr const & other ) {
    > copy_ptr dummy ( other );
    > swap( *this, dummy );
    > return( *this );
    > }
    >
    > T const * operator-> ( void ) const {
    > return( raw_ptr );
    > }
    >
    > T * operator-> ( void ) {
    > return( raw_ptr );
    > }
    >
    > T const & operator* ( void ) const {
    > return( *raw_ptr );
    > }
    >
    > T & operator* ( void ) {
    > return( *raw_ptr );
    > }
    >
    > }; // copy_ptr<T>
    >
    > template < typename T >
    > void swap ( copy_ptr< T > & p, copy_ptr< T > & q ) {
    > std::swap( p.raw_ptr, q.raw_ptr );
    > std::swap( p.clone_fct, p.clone_fct );
    > }
    >
    >
    > // a sanity check:
    >
    > #include <iostream>
    >
    > struct Base {
    >
    > Base ( void ) {
    > std::cout << "base is born.\n";
    > }
    >
    > Base ( Base const & other ) {
    > std::cout << "base is copied.\n";
    > }
    >
    > virtual ~Base () {
    > std::cout << "base dies.\n";
    > }
    >
    > virtual
    > std::eek:stream & dump ( std::eek:stream & ostr ) const {
    > return( ostr << "base" );
    > }
    >
    > };
    >
    > std::eek:stream & operator<< ( std::eek:stream & ostr,
    > Base const & obj ) {
    > return( obj.dump( ostr ) );
    > }
    >
    > struct Derived : public Base {
    >
    > Derived ( void ) {
    > std::cout << "derived is born.\n";
    > }
    >
    > Derived ( Derived const & other )
    > : Base ( other )
    > {
    > std::cout << "derived is copied.\n";
    > }
    >
    > virtual ~Derived () {
    > std::cout << "derived dies.\n";
    > }
    >
    > virtual
    > std::eek:stream & dump ( std::eek:stream & ostr ) const {
    > return( ostr << "derived" );
    > }
    >
    > };
    >


    struct foo { virtual ~foo() {} };

    > int main ( void ) {
    > copy_ptr< Base > a_ptr;
    > copy_ptr< Base > b_ptr ( new Derived() );
    >


    copy_ptr< Base > c_ptr ( new foo() );

    > std::cout << '\n' << *a_ptr;
    > std::cout << " " << *b_ptr << "\n\n";
    >
    > a_ptr = b_ptr;
    > std::cout << '\n' << *a_ptr;
    > std::cout << " " << *b_ptr << "\n\n";
    >
    > }


    not tested, but i believe my additions will compile, but cause
    undefined behavior at runtime. Checking the return of dynamic_cast is
    one solution, but that only identifies the problem at runtime.
    Compile-time type safety would probably be desired. Also, dynamic_cast
    will barf on non-polymorphic types. While using copy_ptr<int>, etc
    would be rather pointless, it would be nice to be allowed for
    consistancy.

    We can avoid the dynamic_cast by maintaining a D pointer in addition to
    or instead of the T pointer. Probably easiest to show with code.

    template<class T>
    class copy_ptr {

    class ptr_holder_interface {
    public:
    // ..
    virtual ptr_holder_interface *clone() const = 0;
    virtual T *get() const = 0;

    /* Or, instead of the above get function (purely time-space trade-off
    optimization):
    T *get() const { return ptr; }

    private:
    T *ptr;
    **/
    };

    template<class D>
    class ptr_holder : public ptr_holder_interface {
    public:
    // ..
    ptr_holder_interface *clone() const { return new
    ptr_holder<D>(d_ptr); }

    private:
    D *d_ptr;
    };

    public:
    template<class D>
    copy_ptr(D *p) : holder(new ptr_holder<D>(p)) {}

    // ...

    private:
    ptr_holder_interface *holder;
    };
     
    Alipha, Aug 22, 2005
    #2
    1. Advertising

  3. Kai-Uwe Bux

    Kai-Uwe Bux Guest

    Many many thanks!

    Alipha wrote:
    [snip]
    > struct foo { virtual ~foo() {} };
    >
    >> int main ( void ) {
    >> copy_ptr< Base > a_ptr;
    >> copy_ptr< Base > b_ptr ( new Derived() );
    >>

    >
    > copy_ptr< Base > c_ptr ( new foo() );
    >
    >> std::cout << '\n' << *a_ptr;
    >> std::cout << " " << *b_ptr << "\n\n";
    >>
    >> a_ptr = b_ptr;
    >> std::cout << '\n' << *a_ptr;
    >> std::cout << " " << *b_ptr << "\n\n";
    >>
    >> }

    >
    > not tested, but i believe my additions will compile, but cause
    > undefined behavior at runtime. Checking the return of dynamic_cast is
    > one solution, but that only identifies the problem at runtime.
    > Compile-time type safety would probably be desired.


    Actually, one cool thing about dynamic_cast<> is that it does those checks
    at compile time.


    > Also, dynamic_cast
    > will barf on non-polymorphic types. While using copy_ptr<int>, etc
    > would be rather pointless, it would be nice to be allowed for
    > consistancy.


    Very good catch!

    > We can avoid the dynamic_cast by maintaining a D pointer in addition to
    > or instead of the T pointer. Probably easiest to show with code.
    >
    > template<class T>
    > class copy_ptr {
    >
    > class ptr_holder_interface {
    > public:
    > // ..
    > virtual ptr_holder_interface *clone() const = 0;
    > virtual T *get() const = 0;
    >
    > /* Or, instead of the above get function (purely time-space trade-off
    > optimization):
    > T *get() const { return ptr; }
    >
    > private:
    > T *ptr;
    > **/
    > };
    >
    > template<class D>
    > class ptr_holder : public ptr_holder_interface {
    > public:
    > // ..
    > ptr_holder_interface *clone() const { return new
    > ptr_holder<D>(d_ptr); }
    >
    > private:
    > D *d_ptr;
    > };
    >
    > public:
    > template<class D>
    > copy_ptr(D *p) : holder(new ptr_holder<D>(p)) {}
    >
    > // ...
    >
    > private:
    > ptr_holder_interface *holder;
    > };


    I had something like that in an earlier version. But, I thought, that way
    you need to do some explicit concept checking at compile time to see
    whether D is derived from T.


    However, about the non-polymorphic types you are absolutely right. Here is
    an idea how to deal with that: (just provide a special clone_function for
    the case D=T.)

    // copy_ptr.cc (C) Kai-Uwe Bux [2005]
    // ==================================

    /*
    | - Upon construction, copy_ptr<T> takes pointee ownership
    | a D* where D is a type derived from T. (compile type check)
    | - In any copy construction or assignment a copy of the
    | pointee is created. There should be no slicing.
    | - Upon destruction the pointee is destroyed.
    | - Intended use is within STL containers.
    | - If T does not have a virtual destructor, copy_ptr<T>
    | cannot be used polymorphically (compile time check).
    */

    // we swap:
    #include <algorithm>

    // The clone_traits function:
    template < typename T, typename D >
    T * clone ( T * ptr ) {
    return( new D ( *( dynamic_cast<D*>( ptr ) ) ) );
    }

    template < typename T >
    T * simple_clone ( T * ptr ) {
    return( new T ( *ptr ) );
    }


    // forward declarations:
    template < typename T >
    class copy_ptr;

    template < typename T >
    void swap ( copy_ptr< T > &, copy_ptr< T > & );


    // implementation:
    template < typename T >
    class copy_ptr {

    friend void swap<> ( copy_ptr<T> &, copy_ptr<T> & );

    /*
    The idea is that in addition to a pointer, we also need
    a pointer to the appropriate clone_traits function.
    */
    T * raw_ptr;
    T * ( *clone__fct ) ( T * );

    public:

    copy_ptr ( void )
    : raw_ptr ( new T )
    , clone__fct( simple_clone<T> )
    {}

    copy_ptr ( T * ptr )
    : raw_ptr ( ptr )
    , clone__fct ( simple_clone<T> )
    {}

    template < typename D >
    copy_ptr ( D * ptr )
    : raw_ptr ( ptr )
    , clone__fct ( clone<T,D> )
    {}

    // copy construction clone_traitss:
    copy_ptr ( copy_ptr const & other )
    : raw_ptr ( other.clone__fct( other.raw_ptr ) )
    , clone__fct ( other.clone__fct )
    {}

    // destruction frees the pointee
    ~copy_ptr ( void ) {
    delete( raw_ptr );
    }

    // assignment reduces to copy construction:
    copy_ptr & operator= ( copy_ptr const & other ) {
    copy_ptr dummy ( other );
    swap( *this, dummy );
    return( *this );
    }

    T const * operator-> ( void ) const {
    return( raw_ptr );
    }

    T * operator-> ( void ) {
    return( raw_ptr );
    }

    T const & operator* ( void ) const {
    return( *raw_ptr );
    }

    T & operator* ( void ) {
    return( *raw_ptr );
    }

    }; // copy_ptr<T>

    template < typename T >
    void swap ( copy_ptr< T > & p, copy_ptr< T > & q ) {
    std::swap( p.raw_ptr, q.raw_ptr );
    std::swap( p.clone__fct, p.clone__fct );
    }


    // a sanity check:

    #include <iostream>

    struct Base {

    Base ( void ) {
    std::cout << "base is born.\n";
    }

    Base ( Base const & other ) {
    std::cout << "base is copied.\n";
    }

    virtual ~Base () {
    std::cout << "base dies.\n";
    }

    virtual
    std::eek:stream & dump ( std::eek:stream & ostr ) const {
    return( ostr << "base" );
    }

    };

    std::eek:stream & operator<< ( std::eek:stream & ostr,
    Base const & obj ) {
    return( obj.dump( ostr ) );
    }

    struct Derived : public Base {

    Derived ( void ) {
    std::cout << "derived is born.\n";
    }

    Derived ( Derived const & other )
    : Base ( other )
    {
    std::cout << "derived is copied.\n";
    }

    virtual ~Derived () {
    std::cout << "derived dies.\n";
    }

    virtual
    std::eek:stream & dump ( std::eek:stream & ostr ) const {
    return( ostr << "derived" );
    }

    };

    struct NotDerived {

    NotDerived ( void ) {
    std::cout << "not-derived is born.\n";
    }

    NotDerived ( NotDerived const & other )
    {
    std::cout << "not-derived is copied.\n";
    }

    virtual ~NotDerived () {
    std::cout << "not-derived dies.\n";
    }

    };

    struct BadBase {};
    struct BadDerived : public BadBase {};

    std::eek:stream & operator<< ( std::eek:stream & ostr, NotDerived const & obj ) {
    return( ostr << "not-derived" );
    }

    int main ( void ) {
    copy_ptr< Base > a_ptr;
    copy_ptr< Base > b_ptr ( new Derived() );

    std::cout << '\n' << *a_ptr;
    std::cout << " " << *b_ptr << "\n\n";

    a_ptr = b_ptr;
    std::cout << '\n' << *a_ptr;
    std::cout << " " << *b_ptr << "\n\n";

    copy_ptr< int > i_ptr;

    // compile time errors:
    // copy_ptr< Base > x_ptr ( new NotDerived() );
    // copy_ptr< int > j_ptr ( new Base() );
    // copy_ptr< BadBase > bad_ptr ( new BadDerived () );

    }


    Thanks again and
    best regards

    Kai-Uwe Bux
     
    Kai-Uwe Bux, Aug 22, 2005
    #3
  4. Kai-Uwe Bux

    Axter Guest

    Kai-Uwe Bux wrote:
    > Hi folks,
    >
    >
    > in another thread there is some discussion about cloning smart pointers. So
    > I started thinking about it and stumbled upon a rather simple generic
    > implementation. However, it uses dynamic_cast<>, and I am not really
    > familiar with that cast. So, I am not sure whether the code is correct.
    > Please have a look and let me know how to improve this.
    >
    >
    >
    > Sorry for the long post
    >
    > Thanks
    >
    > Kai-Uwe Bux
    >
    >
    >
    >
    > // copy_ptr.cc (C) Kai-Uwe Bux [2005]
    > // ==================================
    >
    > /*
    > | - Upon construction, copy_ptr<T> takes pointee ownership
    > | a D* where D is a type derived from T.
    > | - In any copy construction or assignment a copy of the
    > | pointee is created. There should be no slicing.
    > | - Upon destruction the pointee is destroyed.
    > | - Intended use is within STL containers.
    > */
    >
    > // we swap:
    > #include <algorithm>
    >
    > // The clone function:
    > template < typename T, typename D >
    > T * clone ( T * ptr ) {
    > return( new D ( *( dynamic_cast<D*>( ptr ) ) ) );
    > }
    >
    > // forward declarations:
    > template < typename T >
    > class copy_ptr;
    >
    > template < typename T >
    > void swap ( copy_ptr< T > &, copy_ptr< T > & );
    >
    >
    > // implementation:
    > template < typename T >
    > class copy_ptr {
    >
    > friend void swap<> ( copy_ptr<T> &, copy_ptr<T> & );
    >
    > /*
    > The idea is that in addition to a pointer, we also need
    > a pointer to the appropriate clone function.
    > */
    > T * raw_ptr;
    > T * ( *clone_fct ) ( T * );
    >
    > public:
    >
    > copy_ptr ( void )
    > : raw_ptr ( new T )
    > , clone_fct( clone<T,T> )
    > {}
    >
    > template < typename D >
    > copy_ptr ( D * ptr )
    > : raw_ptr ( ptr )
    > , clone_fct ( clone<T,D> )
    > {}
    >
    > // copy construction clones:
    > copy_ptr ( copy_ptr const & other )
    > : raw_ptr ( other.clone_fct( other.raw_ptr ) )
    > , clone_fct ( other.clone_fct )
    > {}
    >
    > // destruction frees the pointee
    > ~copy_ptr ( void ) {
    > delete( raw_ptr );
    > }
    >
    > // assignment reduces to copy construction:
    > copy_ptr & operator= ( copy_ptr const & other ) {
    > copy_ptr dummy ( other );
    > swap( *this, dummy );
    > return( *this );
    > }
    >
    > T const * operator-> ( void ) const {
    > return( raw_ptr );
    > }
    >
    > T * operator-> ( void ) {
    > return( raw_ptr );
    > }
    >
    > T const & operator* ( void ) const {
    > return( *raw_ptr );
    > }
    >
    > T & operator* ( void ) {
    > return( *raw_ptr );
    > }
    >
    > }; // copy_ptr<T>
    >
    > template < typename T >
    > void swap ( copy_ptr< T > & p, copy_ptr< T > & q ) {
    > std::swap( p.raw_ptr, q.raw_ptr );
    > std::swap( p.clone_fct, p.clone_fct );
    > }
    >
    >
    > // a sanity check:
    >
    > #include <iostream>
    >
    > struct Base {
    >
    > Base ( void ) {
    > std::cout << "base is born.\n";
    > }
    >
    > Base ( Base const & other ) {
    > std::cout << "base is copied.\n";
    > }
    >
    > virtual ~Base () {
    > std::cout << "base dies.\n";
    > }
    >
    > virtual
    > std::eek:stream & dump ( std::eek:stream & ostr ) const {
    > return( ostr << "base" );
    > }
    >
    > };
    >
    > std::eek:stream & operator<< ( std::eek:stream & ostr,
    > Base const & obj ) {
    > return( obj.dump( ostr ) );
    > }
    >
    > struct Derived : public Base {
    >
    > Derived ( void ) {
    > std::cout << "derived is born.\n";
    > }
    >
    > Derived ( Derived const & other )
    > : Base ( other )
    > {
    > std::cout << "derived is copied.\n";
    > }
    >
    > virtual ~Derived () {
    > std::cout << "derived dies.\n";
    > }
    >
    > virtual
    > std::eek:stream & dump ( std::eek:stream & ostr ) const {
    > return( ostr << "derived" );
    > }
    >
    > };
    >
    > int main ( void ) {
    > copy_ptr< Base > a_ptr;
    > copy_ptr< Base > b_ptr ( new Derived() );
    >
    > std::cout << '\n' << *a_ptr;
    > std::cout << " " << *b_ptr << "\n\n";
    >
    > a_ptr = b_ptr;
    > std::cout << '\n' << *a_ptr;
    > std::cout << " " << *b_ptr << "\n\n";
    >
    > }


    That's a pretty slick method. I would modify it to use allocator
    types, so the clone can be done via custom allocators if required.
     
    Axter, Aug 23, 2005
    #4
  5. Kai-Uwe Bux

    Kai-Uwe Bux Guest

    Axter wrote:

    >
    > Kai-Uwe Bux wrote:


    >> // The clone function:
    >> template < typename T, typename D >
    >> T * clone ( T * ptr ) {
    >> return( new D ( *( dynamic_cast<D*>( ptr ) ) ) );
    >> }

    [snip]
    >
    >> // implementation:
    >> template < typename T >
    >> class copy_ptr {
    >>
    >> friend void swap<> ( copy_ptr<T> &, copy_ptr<T> & );
    >>
    >> /*
    >> The idea is that in addition to a pointer, we also need
    >> a pointer to the appropriate clone function.
    >> */
    >> T * raw_ptr;
    >> T * ( *clone_fct ) ( T * );
    >>
    >> public:
    >>
    >> copy_ptr ( void )
    >> : raw_ptr ( new T )
    >> , clone_fct( clone<T,T> )
    >> {}
    >>
    >> template < typename D >
    >> copy_ptr ( D * ptr )
    >> : raw_ptr ( ptr )
    >> , clone_fct ( clone<T,D> )
    >> {}
    >>
    >> // copy construction clones:
    >> copy_ptr ( copy_ptr const & other )
    >> : raw_ptr ( other.clone_fct( other.raw_ptr ) )
    >> , clone_fct ( other.clone_fct )
    >> {}
    >>
    >> // destruction frees the pointee
    >> ~copy_ptr ( void ) {
    >> delete( raw_ptr );
    >> }
    >>
    >> // assignment reduces to copy construction:
    >> copy_ptr & operator= ( copy_ptr const & other ) {
    >> copy_ptr dummy ( other );
    >> swap( *this, dummy );
    >> return( *this );
    >> }
    >>
    >> T const * operator-> ( void ) const {
    >> return( raw_ptr );
    >> }
    >>
    >> T * operator-> ( void ) {
    >> return( raw_ptr );
    >> }
    >>
    >> T const & operator* ( void ) const {
    >> return( *raw_ptr );
    >> }
    >>
    >> T & operator* ( void ) {
    >> return( *raw_ptr );
    >> }
    >>
    >> }; // copy_ptr<T>
    >>


    > That's a pretty slick method. I would modify it to use allocator
    > types, so the clone can be done via custom allocators if required.


    Many thanks!

    That is a very good suggestion, and I started thinking about it. One way to
    go about it is pretty straight forward: turn clone<T,D>() into
    clone<T,D,Alloc>(). However, I think in the destructor, the call to delete
    would need to be replaced by the appropriate Alloc::deallocate() call
    (after rebinding the allocator to the right type. The most natural way I
    see right now is to add a template

    template < typename T, typename D, typename Alloc >
    void kill ( T* ptr ) {
    // let's think about syntax later:
    /*
    destruct the object
    use Alloc<D>::deallocate( dynamic_cast<D*>( ptr ) );
    */
    }

    and store a pointer to the appropriate kill() in the copy_ptr<T,Alloc>
    object. But I might be stupidly trying to use the same trick twice. It
    would be nice, though, if one could avoid this additional field.

    In any case, thanks a lot for your comment. I will try that out and post
    some version soon.


    Best regards

    Kai-Uwe Bux
     
    Kai-Uwe Bux, Aug 23, 2005
    #5
  6. In article <dedbju$8u$>,
    Kai-Uwe Bux <> wrote:

    > // copy_ptr.cc (C) Kai-Uwe Bux [2005]
    > // ==================================


    Beware of this use case:

    struct Derived2 : public Derived {

    Derived2 ( void ) {
    std::cout << "derived2 is born.\n";
    }

    Derived2 ( Derived2 const & other )
    : Derived ( other )
    {
    std::cout << "derived2 is copied.\n";
    }

    virtual ~Derived2 () {
    std::cout << "derived2 dies.\n";
    }

    virtual
    std::eek:stream & dump ( std::eek:stream & ostr ) const {
    return( ostr << "derived2" );
    }

    };

    ....

    Derived* d2p = new Derived;
    copy_ptr< Base > d2_ptr ( d2p );
    copy_ptr< Base > d2_ptr_also(d2_ptr);

    The copy slices your Derived2 down to a Derived.

    A more bullet-proof approach (imho) is to have a copy_ptr which simply
    does (untested):

    copy_ptr ( copy_ptr const & other )
    : raw_ptr (0)
    {
    if (other.raw_ptr)
    raw_ptr = new T(*other);
    }

    And also a separate clone_ptr:

    clone_ptr (clone_ptr const & other )
    : raw_ptr (0)
    {
    if (other.raw_ptr)
    raw_ptr = other->clone();
    }

    I.e. Two fundamental tools instead of trying to merge them under one
    roof. Lower overhead, simpler, easier concept for clients to grasp.

    I also note that null testing is missing. And you've implemented deep
    const. Some people really like deep const. Other's really dislike it.
    Standard and proposed standard smart pointers implement shallow const.
    Document it loudly. :)

    -Howard
     
    Howard Hinnant, Aug 23, 2005
    #6
  7. Kai-Uwe Bux

    Kai-Uwe Bux Guest

    Howard Hinnant wrote:

    > In article <dedbju$8u$>,
    > Kai-Uwe Bux <> wrote:
    >
    >> // copy_ptr.cc (C) Kai-Uwe Bux [2005]
    >> // ==================================

    >
    > Beware of this use case:
    >
    > struct Derived2 : public Derived {
    >
    > Derived2 ( void ) {
    > std::cout << "derived2 is born.\n";
    > }
    >
    > Derived2 ( Derived2 const & other )
    > : Derived ( other )
    > {
    > std::cout << "derived2 is copied.\n";
    > }
    >
    > virtual ~Derived2 () {
    > std::cout << "derived2 dies.\n";
    > }
    >
    > virtual
    > std::eek:stream & dump ( std::eek:stream & ostr ) const {
    > return( ostr << "derived2" );
    > }
    >
    > };
    >
    > ...
    >
    > Derived* d2p = new Derived;
    > copy_ptr< Base > d2_ptr ( d2p );
    > copy_ptr< Base > d2_ptr_also(d2_ptr);
    >
    > The copy slices your Derived2 down to a Derived.


    You are right, copy_ptr<> is dangerous:

    As for slicing, the problem is not a 2-step derivation, already

    Base * b_ptr = new Derived ( ... );
    copy_ptr< Base > box ( b_ptr );

    will happily slice the object.

    The copy_ptr<> template should be handled by strictly conforming to certain
    idioms. In particular, it should always be initialized like so:

    copy_ptr< T > ptr ( new D ( ... ) );

    This idiom is the intended use. Whenever people depart, BadThings(tm) will
    hurt you. [Reminds me of std::auto_ptr<>.] This is somewhat by design (you
    also noted the missing check for NULL in the constructor -- well, it is
    intentional).

    Now, if I really wanted to tie the clients into some kind of scheme, I would
    provide different constructors:

    copy_ptr<T> ( T ); // beware of slicing!

    and

    copy_ptr<T> ( copy_ptr<D> const & );


    and then all copy pointers would be initialized by copy-construction from
    pre-existing objects. This way, the slicing is well documented and
    non-surprising and NULL initialization is impossible.


    > A more bullet-proof approach (imho) is to have a copy_ptr which simply
    > does (untested):
    >
    > copy_ptr ( copy_ptr const & other )
    > : raw_ptr (0)
    > {
    > if (other.raw_ptr)
    > raw_ptr = new T(*other);
    > }
    >


    Actually such a copy_ptr<T> does not offer anything that T does not do
    already.

    > And also a separate clone_ptr:
    >
    > clone_ptr (clone_ptr const & other )
    > : raw_ptr (0)
    > {
    > if (other.raw_ptr)
    > raw_ptr = other->clone();
    > }
    >
    > I.e. Two fundamental tools instead of trying to merge them under one
    > roof. Lower overhead, simpler, easier concept for clients to grasp.


    I think, the overhead of a clone pointer is not lesser, just hidden in the
    management of virtual function calls. As for being more simple and an
    easier concept from a client perspective, I have no opinion since I lack
    experience with clone pointers.



    Best

    Kai-Uwe Bux


    ps.: What I *really* want is a polymorphic variable (realized via a smart
    reference not a smart pointer). Unfortunately, one cannot overload the
    dot-operator. Oh well.
     
    Kai-Uwe Bux, Aug 24, 2005
    #7
  8. In article <deii8p$hvo$>,
    Kai-Uwe Bux <> wrote:

    > As for slicing, the problem is not a 2-step derivation, already
    >
    > Base * b_ptr = new Derived ( ... );
    > copy_ptr< Base > box ( b_ptr );
    >
    > will happily slice the object.


    Yup.

    > The copy_ptr<> template should be handled by strictly conforming to certain
    > idioms. In particular, it should always be initialized like so:
    >
    > copy_ptr< T > ptr ( new D ( ... ) );
    >
    > This idiom is the intended use. Whenever people depart, BadThings(tm) will
    > hurt you. [Reminds me of std::auto_ptr<>.]


    :)

    > This is somewhat by design (you
    > also noted the missing check for NULL in the constructor -- well, it is
    > intentional).


    Actually the NULL check I was talking about was for this kind of support:

    copy_ptr<T> p;
    ....
    if (p) ...

    or

    if (p == NULL)

    It can safely be accomplished with a pointer to a private data member.
    See:

    http://home.twcny.rr.com/hinnant/cpp_extensions/unique_ptr.html

    and search for "nat" and:

    operator int nat::*() const {return ptr_.first() ? &nat::for_bool_ : 0;}

    > > A more bullet-proof approach (imho) is to have a copy_ptr which simply
    > > does (untested):
    > >
    > > copy_ptr ( copy_ptr const & other )
    > > : raw_ptr (0)
    > > {
    > > if (other.raw_ptr)
    > > raw_ptr = new T(*other);
    > > }
    > >

    >
    > Actually such a copy_ptr<T> does not offer anything that T does not do
    > already.


    <nod> You may be right (I've never needed one either). So why bother
    with:

    template < typename T >
    T * simple_clone ( T * ptr ) {
    return( new T ( *ptr ) );
    }

    Does that not turn your copy_ptr into my copy_ptr for a certain class of
    types?

    > > And also a separate clone_ptr:
    > >
    > > clone_ptr (clone_ptr const & other )
    > > : raw_ptr (0)
    > > {
    > > if (other.raw_ptr)
    > > raw_ptr = other->clone();
    > > }
    > >
    > > I.e. Two fundamental tools instead of trying to merge them under one
    > > roof. Lower overhead, simpler, easier concept for clients to grasp.

    >
    > I think, the overhead of a clone pointer is not lesser, just hidden in the
    > management of virtual function calls. As for being more simple and an
    > easier concept from a client perspective, I have no opinion since I lack
    > experience with clone pointers.


    I think the overhead is lesser. My suggested clone_ptr need only hold
    the pointer. And the pointee need only have a virtual destructor and
    virtual clone() function.

    Your copy_ptr requires two pointers, and the pointee still needs a
    virtual function table (else the dynamic_cast will fail).

    Your copy_ptr has less restrictive requirements on the pointee (which is
    a good thing). The pointee need not implement a virtual copy
    constructor named clone().

    My clone_ptr variant drops the internal function pointer by requiring
    the clone() signature of the pointee. <shrug> it's a tradeoff. But
    lots of generic code requires that the template parameter meet some
    requirements (or concepts). E.g. std::for_each:

    template<class InputIterator, class Function>
    Function
    for_each(InputIterator first, InputIterator last, Function f);

    Requires InputIterator to meet all of the requirements of an input
    iterator. Also requires of Function that the following be well defined:

    f(*first);

    Similarly clone_ptr<T> would require that:

    T* p1 = ... // p1 non-null
    T* p2 = p1->clone();

    be well defined and that the dynamic type of *p1 and *p2 be the same,
    and that *p2 be a copy of *p1.

    Failure to have a clone() signature would of course be caught at compile
    time. Having a clone() signature with other semantics would be
    problematic and probably result in a run time failure. So major
    questions are: Is clone() sufficiently universal in it's meaning (like
    swap(x,y) has become)? Is requiring it of your pointee overly
    burdensome?

    I'm inclined to answer yes and no, but freely admit that this is a
    subjective response.

    > ps.: What I *really* want is a polymorphic variable (realized via a smart
    > reference not a smart pointer). Unfortunately, one cannot overload the
    > dot-operator. Oh well.


    This has been discussed in committee, but I'm not sure of the current
    status. There are more language proposals than there are people to work
    on them.

    -Howard
     
    Howard Hinnant, Aug 25, 2005
    #8
  9. Kai-Uwe Bux

    Axter Guest

    Kai-Uwe Bux wrote:
    > Axter wrote:
    >
    > >
    > > Kai-Uwe Bux wrote:

    >
    > >> // The clone function:
    > >> template < typename T, typename D >
    > >> T * clone ( T * ptr ) {
    > >> return( new D ( *( dynamic_cast<D*>( ptr ) ) ) );
    > >> }

    > [snip]
    > >
    > >> // implementation:
    > >> template < typename T >
    > >> class copy_ptr {
    > >>
    > >> friend void swap<> ( copy_ptr<T> &, copy_ptr<T> & );
    > >>
    > >> /*
    > >> The idea is that in addition to a pointer, we also need
    > >> a pointer to the appropriate clone function.
    > >> */
    > >> T * raw_ptr;
    > >> T * ( *clone_fct ) ( T * );
    > >>
    > >> public:
    > >>
    > >> copy_ptr ( void )
    > >> : raw_ptr ( new T )
    > >> , clone_fct( clone<T,T> )
    > >> {}
    > >>
    > >> template < typename D >
    > >> copy_ptr ( D * ptr )
    > >> : raw_ptr ( ptr )
    > >> , clone_fct ( clone<T,D> )
    > >> {}
    > >>
    > >> // copy construction clones:
    > >> copy_ptr ( copy_ptr const & other )
    > >> : raw_ptr ( other.clone_fct( other.raw_ptr ) )
    > >> , clone_fct ( other.clone_fct )
    > >> {}
    > >>
    > >> // destruction frees the pointee
    > >> ~copy_ptr ( void ) {
    > >> delete( raw_ptr );
    > >> }
    > >>
    > >> // assignment reduces to copy construction:
    > >> copy_ptr & operator= ( copy_ptr const & other ) {
    > >> copy_ptr dummy ( other );
    > >> swap( *this, dummy );
    > >> return( *this );
    > >> }
    > >>
    > >> T const * operator-> ( void ) const {
    > >> return( raw_ptr );
    > >> }
    > >>
    > >> T * operator-> ( void ) {
    > >> return( raw_ptr );
    > >> }
    > >>
    > >> T const & operator* ( void ) const {
    > >> return( *raw_ptr );
    > >> }
    > >>
    > >> T & operator* ( void ) {
    > >> return( *raw_ptr );
    > >> }
    > >>
    > >> }; // copy_ptr<T>
    > >>

    >
    > > That's a pretty slick method. I would modify it to use allocator
    > > types, so the clone can be done via custom allocators if required.

    >
    > Many thanks!
    >
    > That is a very good suggestion, and I started thinking about it. One way to
    > go about it is pretty straight forward: turn clone<T,D>() into
    > clone<T,D,Alloc>(). However, I think in the destructor, the call to delete
    > would need to be replaced by the appropriate Alloc::deallocate() call
    > (after rebinding the allocator to the right type. The most natural way I
    > see right now is to add a template
    >
    > template < typename T, typename D, typename Alloc >
    > void kill ( T* ptr ) {
    > // let's think about syntax later:
    > /*
    > destruct the object
    > use Alloc<D>::deallocate( dynamic_cast<D*>( ptr ) );
    > */
    > }
    >
    > and store a pointer to the appropriate kill() in the copy_ptr<T,Alloc>
    > object. But I might be stupidly trying to use the same trick twice. It
    > would be nice, though, if one could avoid this additional field.
    >
    > In any case, thanks a lot for your comment. I will try that out and post
    > some version soon.
    >
    >
    > Best regards
    >
    > Kai-Uwe Bux


    I was playing around with this method, and one problem I had with it,
    is that I couldn't get it to compile on VC++ 6.0 or Borland BCC55.
    See following link for class and demo project.
    http://code.axter.com/copy_ptr.h
    http://code.axter.com/clone_ptr_demo.zip

    It works just fine in VC++ 7.1 and GNU 3.x
     
    Axter, Aug 27, 2005
    #9
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Code Review Request

    , Jun 30, 2005, in forum: Java
    Replies:
    2
    Views:
    561
    Andrew Thompson
    Jun 30, 2005
  2. P Kenter

    Request for code review

    P Kenter, May 28, 2004, in forum: C++
    Replies:
    4
    Views:
    375
    P Kenter
    Jun 2, 2004
  3. Ben Hanson

    Request for Code Review

    Ben Hanson, Jul 2, 2004, in forum: C++
    Replies:
    19
    Views:
    688
    Chris Gordon-Smith
    Jul 4, 2004
  4. Feniks
    Replies:
    5
    Views:
    669
    Feniks
    Nov 11, 2006
  5. www
    Replies:
    51
    Views:
    1,517
Loading...

Share This Page