copy_ptr: request for code review.

K

Kai-Uwe Bux

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";

}
 
A

Alipha

Kai-Uwe Bux said:
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 said:
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;
};
 
K

Kai-Uwe Bux

Many many thanks!

Alipha wrote:
[snip]
struct foo { virtual ~foo() {} };




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
 
A

Axter

Kai-Uwe Bux said:
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.
 
K

Kai-Uwe Bux

Axter said:
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
 
H

Howard Hinnant

Kai-Uwe Bux said:
// 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
 
K

Kai-Uwe Bux

Howard said:
Kai-Uwe Bux said:
// 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);
}


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.
 
H

Howard Hinnant

Kai-Uwe Bux said:
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;}
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?
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
 
A

Axter

Kai-Uwe Bux said:
Axter said:
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
 

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,995
Messages
2,570,236
Members
46,822
Latest member
israfaceZa

Latest Threads

Top