Comments on value_ptr class?

I'm revisting an idea that was brought up a few days ago about a non-slicing smart-pointer.

It occurred to me that this might be a pretty useful addition to the STL, and so I was hoping that some of you here might be interested in helping to develop it into something that could eventually be submitted as a recommendation to the C++ standards commitee?

At any rate, all comments and suggestions are welcome. Here is the current implementation:

Code:

#include <memory > // for std::allocator
#include <algorithm > // for std::swap

/*
	A simple "non-slicing" memory manager. 
	Note: NOT to be used to manage dynamically-allocated 
	arrays (use std::vector or similar for arrays)
*/
template < typename Type, typename Allocator = std::allocator< Type > > 
class value_ptr 
{
	protected:
	
	class internal_allocator_base
	{
		public:
		
		virtual Type* clone( Type const* ) const = 0;
		
		virtual void destroy( Type* ) const = 0;
	};
	
	template < typename Derived >
	class internal_allocator : public internal_allocator_base
	{
		public:	
	
		virtual Type* clone( Type const* ptr ) const
		{
			if( ptr == 0 )
				return 0;
			Derived* 
				val = allocator.allocate( 1 );
			try
			{
				allocator.construct( val, *static_cast< Derived const* >( ptr ) );
			}
			catch( ... )
			{
				allocator.deallocate( val, 1 );
				throw;
			}
			return val;
		}		
		
		virtual void destroy( Type* ptr ) const
		{
			if( ptr != 0 )
			{
				Derived* 
					tmp = static_cast< Derived* >( ptr );
				try
				{
					allocator.destroy( tmp );
				}
				catch( ... )
				{
					allocator.deallocate( tmp, 1 );
					throw;
				}				
				allocator.deallocate( tmp, 1 );			
			}	
		}		
		
		static internal_allocator 
			global;
		static typename Allocator::template rebind< Derived >::other 
			allocator;		
	};

	public:

	template < typename Derived >
	value_ptr( Derived* ptr )
	{	
		init( );
		reset( ptr );
	}
	
	value_ptr( Type* ptr = 0 )
	{	
		init( );
		reset( ptr );
	}	

 	value_ptr( value_ptr const& rhs )
	{	
		init( );
		reset( rhs );
	}
	
	template < typename Derived >
	value_ptr& reset( Derived* ptr )
	{
		internal_reset( ptr );
		m_iab_ = &internal_allocator< Derived >::global;
		return *this;
	}
	
	template < typename Derived >
	inline value_ptr& operator = ( Derived* ptr )
	{
		return reset( ptr );
	}
	
	inline value_ptr& reset( Type* ptr = 0 )
	{
		return reset< Type >( ptr );
	}
	
 	inline value_ptr& operator = ( Type* ptr )
	{
		return reset( ptr );
	}		
	
 	value_ptr& reset( value_ptr const& rhs )
	{		
		internal_reset( rhs.m_iab_->clone( rhs.m_ptr_ ) );		
		m_iab_ = rhs.m_iab_;
		return *this;
	}

	inline value_ptr& operator = ( value_ptr const& rhs )
	{	
		return reset( rhs );
	}
	
	template < typename Derived >
	value_ptr& allocate( Derived const& val = Derived( ) )
	{
		return reset
		( 
			static_cast< Derived* >
			( 
				internal_allocator< Derived >::global.clone( &val ) 
			)
		);
	}

	inline value_ptr& allocate( void )
	{
		return allocate< Type >( );
	}	
	
	friend void swap( value_ptr& lhs, value_ptr& rhs )
	{
		std::swap( lhs.m_ptr_, rhs.m_ptr_ );
		std::swap( lhs.m_iab_, rhs.m_iab_ );
	}

#ifdef VALUE_PTR_NO_IMPLICIT_CONVERSION
	inline Type& operator * ( void )
	{
		return *m_ptr_;
	}

	inline Type const& operator * ( void ) const
	{
		return *m_ptr_;
	}
#else 	
	inline operator Type* ( void )
	{
		return m_ptr_;
	}

	inline operator Type const* ( void ) const
	{
		return m_ptr_;
	}
#endif
	
	inline Type* operator -> ( void )
	{
		return m_ptr_;
	}

	inline Type const* operator -> ( void ) const
	{
		return m_ptr_;
	}
		
	virtual ~value_ptr( void )
	{
		reset( );
	}	
 
	protected:
	
	void init( void )
	{
		m_ptr_ = 0;
		m_iab_ = &internal_allocator< Type >::global;
	}	
	
	void internal_reset( Type* ptr )
	{
	/*
		The temporary ensures that if the 'current' 
		data's destructor throws an exception, 
		the 'new' memory will still be cleaned up.
	*/	
		Type* 
			saved = m_ptr_;
		m_ptr_ = ptr;
		m_iab_->destroy( saved );
	}

	Type* 
		m_ptr_;
	internal_allocator_base* 
		m_iab_;
};

template < typename Type, typename Allocator > 
template < typename Derived > 
typename value_ptr< Type, Allocator >::template internal_allocator< Derived >
	value_ptr< Type, Allocator >::internal_allocator< Derived >::global 
	= typename value_ptr< Type, Allocator >::template internal_allocator< Derived >( ); 	
template < typename Type, typename Allocator > 
template < typename Derived > 
typename Allocator::template rebind< Derived >::other 
	value_ptr< Type, Allocator >::internal_allocator< Derived >::allocator	
	= typename Allocator::template rebind< Derived >::other( );

And a simple test program:

Code:

#include <iostream>

class base
{
    public:
    
    void print( char const* msg )
    {
        std::cout 
        << reinterpret_cast< int* >( this ) 
        << " : " << msg << std::endl;
    }
    
    base( void )
    {
        print( "base::base( )" );
    }
    
    base( base const& )
    {
        print( "base::base( base const& )" );
    }    
    
    virtual void test( void )
    {
        print( "base::test( )" );
    }
    
    ~base( void )
    {
        print( "base::~base( )" );
    }
};

class derived : public base
{
    public:
    
    derived( void )
    {
        print( "derived::derived( )" );
    }

    derived( derived const& rhs )
    : base( rhs )
    {
        print( "derived::derived( derived const& )" );
    }        
    
    virtual void test( void )
    {
        print( "derived::test( )" );
    }
    
    ~derived( void )
    {
        print( "derived::~derived( )" );
    }    
};

int main( void )
{
    value_ptr< base >
        mb1 = new derived( ),      
        mb2 = mb1;       
    mb2->test( );            
    return 0;
}

One thing I couldn't decide on was whether or not to give each instance a copy of it's allocator. It would certainly be a little more flexible (and more in line with other STL objects), but it just seemed so wasteful for a single object, so I've left it out for now.

I definitely think something like that would be useful. I even asked about it on CodeGuru a few years ago.

There has got to be some reason it hasn't been added to boost, yet, though. It is too obvious of a need to just be forgotten about. The difficulty in making a user friendly interface that properly copies would seem to be the problem.

Your test case seems straightforward, but how does value_ptr< base > know about the derived class at all? I wasn't able to figure that out on a quick look.

>> There has got to be some reason it hasn't been added to boost, yet, though. It is too obvious of a need to just be forgotten about. The difficulty in making a user friendly interface that properly copies would seem to be the problem.

I was wondering about that, too, actually. I was worried that there might be some good reason why such a class might be problematic, but so far I haven't been able to think of any.

>> Your test case seems straightforward, but how does value_ptr< base > know about the derived class at all?

Basically, it stores a pointer to a static allocator-type object that 'remembers' the derived type so that it can make copies and destroy the data properly (the latter really shouldn't be necessary since a well-behaved base-class will have a virtual destructor, though).

It uses the internal allocator, which stores the type of the pointer at construction.

However, it breaks down as soon as you deviate from the best practices:

Code:

int main( void )
{
    base* p = new derived;
    value_ptr< base >
        mb1(p),      
        mb2 = mb1;       
    mb2->test( );            
    return 0;
}

I suppose a more "modern" approach (with C++0x) could eliminate new in user code as well: with variadic templates and parameter forwarding you can make a factory function that allocates the instance for the user itself.

Also, it doesn't seem to compile with VC++ (the very thick part).

>> However, it breaks down as soon as you deviate from the best practices:

True, but as long as the proper type is specified during the assignment (via a cast) it works fine. Definitely something to keep in mind as a user, though.

>> I suppose a more "modern" approach (with C++0x) could eliminate new in user code as well: with variadic templates and parameter forwarding you can make a factory function that allocates the instance for the user itself.

Using 'new' directly can absolutely be a dangerous thing, if you aren't careful.

Code:

void foo( std::auto_ptr< int > a, std::auto_ptr< int > b );
// ...
foo( new int, new int );

It's easy to forget that code like that isn't safe. That's basically what the value_ptr::allocate functions are addressing, but yes, it would be really nice to see how that could be C++0x-ified.

>> Also, it doesn't seem to compile with VC++ (the very thick part).

Could you post the line that the error occurs at?

Without having VC++ at the moment, it seems you forgot a "typename" when you initialize the global internal allocator:

Code:

template < typename Type, typename Allocator > 
template < typename Derived > 
typename value_ptr< Type, Allocator >::internal_allocator< Derived >
	value_ptr< Type, Allocator >::internal_allocator< Derived >::global 
	= value_ptr< Type, Allocator >::internal_allocator< Derived >( );

Originally Posted by Ronix

Without having VC++ at the moment, it seems you forgot a "typename" when you initialize the global internal allocator:

Code:

template < typename Type, typename Allocator > 
template < typename Derived > 
typename value_ptr< Type, Allocator >::internal_allocator< Derived >
    value_ptr< Type, Allocator >::internal_allocator< Derived >::global 
    = value_ptr< Type, Allocator >::internal_allocator< Derived >( );

Yep, that was it (I also forgot the template keyword there). Thanks.

Hmm, for some reason the board won't allow me to edit my original post (a new 24-hour rule perhaps?). Here is the current version.

Code:

#include <memory> // for std::allocator
#include <algorithm> // for std::swap

/*
	A simple "non-slicing" memory manager. 
	Note: NOT to be used to manage dynamically-allocated 
	arrays (use std::vector or similar for that).
*/
template < typename Type, typename Allocator = std::allocator< Type > > 
class value_ptr 
{
	protected:
	
	class internal_allocator_base
	{
		public:
		
		virtual Type* clone( Type const* ) const = 0;
		
		virtual void destroy( Type* ) const = 0;
	};
	
	template < typename Derived >
	class internal_allocator : public internal_allocator_base
	{
		public:	
	
		virtual Type* clone( Type const* ptr ) const
		{
			if( ptr == 0 )
				return 0;
			Derived* 
				val = allocator.allocate( 1 );
			try
			{
				allocator.construct
				( 
					val, 
					*static_cast< Derived const* >( ptr ) 
				);
			}
			catch( ... )
			{
				allocator.deallocate( val, 1 );
				throw;
			}
			return val;
		}		
		
		virtual void destroy( Type* ptr ) const
		{
			if( ptr != 0 )
			{
				Derived* 
					tmp = static_cast< Derived* >( ptr );
				try
				{
					allocator.destroy( tmp );
				}
				catch( ... )
				{
					allocator.deallocate( tmp, 1 );
					throw;
				}				
				allocator.deallocate( tmp, 1 );			
			}	
		}		
		
		static internal_allocator 
			global;
		static typename Allocator::template rebind< Derived >::other 
			allocator;		
	};

	public:

	template < typename Derived >
	value_ptr( Derived* ptr )
	{	
		init( );
		reset( ptr );
	}
	
	value_ptr( Type* ptr = 0 )
	{	
		init( );
		reset( ptr );
	}	

 	value_ptr( value_ptr const& rhs )
	{	
		init( );
		reset( rhs );
	}
	
	template < typename Derived >
	value_ptr& reset( Derived* ptr )
	{
		return internal_reset( ptr, &internal_allocator< Derived >::global );
	}
	
	template < typename Derived >
	inline value_ptr& operator = ( Derived* ptr )
	{
		return reset( ptr );
	}
	
	inline value_ptr& reset( Type* ptr = 0 )
	{
		return reset< Type >( ptr );
	}
	
 	inline value_ptr& operator = ( Type* ptr )
	{
		return reset( ptr );
	}		
	
 	value_ptr& reset( value_ptr const& rhs )
	{		
		return internal_reset( rhs.m_iab_->clone( rhs.m_ptr_ ), rhs.m_iab_ );		
	}

	inline value_ptr& operator = ( value_ptr const& rhs )
	{	
		return reset( rhs );
	}
	
	template < typename Derived >
	value_ptr& allocate( Derived const& val = Derived( ) )
	{
		return reset
		( 
			static_cast< Derived* >
			( 
				internal_allocator< Derived >::global.clone( &val ) 
			)
		);
	}

	inline value_ptr& allocate( void )
	{
		return allocate< Type >( );
	}	

#ifdef VALUE_PTR_NO_IMPLICIT_CONVERSION
	inline Type& operator * ( void )
	{
		return *m_ptr_;
	}

	inline Type const& operator * ( void ) const
	{
		return *m_ptr_;
	}
#else 	
	inline operator Type* ( void )
	{
		return m_ptr_;
	}

	inline operator Type const* ( void ) const
	{
		return m_ptr_;
	}
#endif
	
	inline Type* operator -> ( void )
	{
		return m_ptr_;
	}

	inline Type const* operator -> ( void ) const
	{
		return m_ptr_;
	}
	
	friend void swap( value_ptr& lhs, value_ptr& rhs )
	{
		std::swap( lhs.m_ptr_, rhs.m_ptr_ );
		std::swap( lhs.m_iab_, rhs.m_iab_ );
	}

	Type* orphan( void )
	{
		Type*
			tmp = m_ptr_;
		m_ptr_ = 0;
		return tmp;
	}
		
	virtual ~value_ptr( void )
	{
		reset( );
	}	
 
	protected:
	
	void init( void )
	{
		m_ptr_ = 0;
		m_iab_ = &internal_allocator< Type >::global;
	}	
	
	value_ptr& internal_reset( Type* ptr, internal_allocator_base* iab )
	{
	/*
		The swap ensures that if the 'current' 
		data's destructor throws an exception, 
		the 'new' memory will still be cleaned up.
	*/	
		std::swap( m_ptr_, ptr );
		std::swap( m_iab_, iab );	
		iab->destroy( ptr );
		return *this;
	}

	Type* 
		m_ptr_;
	internal_allocator_base* 
		m_iab_;
};

template < typename Type, typename Allocator > 
template < typename Derived > 
typename value_ptr< Type, Allocator >::template internal_allocator< Derived >
	value_ptr< Type, Allocator >::internal_allocator< Derived >::global 
	= typename value_ptr< Type, Allocator >::template internal_allocator< Derived >( ); 	
template < typename Type, typename Allocator > 
template < typename Derived > 
typename Allocator::template rebind< Derived >::other 
	value_ptr< Type, Allocator >::internal_allocator< Derived >::allocator	
	= typename Allocator::template rebind< Derived >::other( );

Code:

   try
      {
      allocator.destroy( tmp );
      }
   catch( ... )
      {
      allocator.deallocate( tmp, 1 );
      throw;
      }

Is this necessary? I imagine that Allocator::destroy should never throw, or at least I'd figure that code that does it is broken and beyond consideration.

Originally Posted by Ronix

Code:

   try
      {
      allocator.destroy( tmp );
      }
   catch( ... )
      {
      allocator.deallocate( tmp, 1 );
      throw;
      }

Is this necessary? I imagine that Allocator::destroy should never throw, or at least I'd figure that code that does it is broken and beyond consideration.

That is a good point, and actually debated it for some time. I wasn't completely sure whether taking the defensive route was really warranted or not (value_ptr::internal_reset also takes similar steps to deal with the situation), but in the end I simply followed my instincts to protect the integrity of the class as a whole. If nothing else, it does help to localize whatever caused the throw-from-destructor event, but then again I do understand the argument that a hopeless situation can't be helped much anyway - so why bother?

Okay, well, I've decided to submit the class as-is (not so much as a complete implementation, but rather a simple 'rough draft'). I really do think that something like this would be a great addition to the standard library, and it would be great to hear from you guys about possible improvements to the design, additional features, etc. If anyone is interested in contributing, just PM me or simply post to this thread.

Thanks.

- Sebastian

Huh? Mr Edd wrote value_ptr two years ago:
mr-edd.co.uk » value_ptr

Is this a derived or modified works, or is it a case of "great minds think alike"?
I did mention value_ptr on this site by name, not so long ago.

In Boost, such a thing usually goes by the name clone_ptr. A search on the developer mailing list reveals several threads on classes of this type, and there even appears to be an implementation in the vault. I didn't take the time to look at specific objections, though. There were some efficiency concerns should such a pointer be stored in standard containers (especially vector), but the same problem exists for std::string and many others.

Originally Posted by iMalc

Huh? Mr Edd wrote value_ptr two years ago:
mr-edd.co.uk » value_ptr

Is this a derived or modified works, or is it a case of "great minds think alike"?
I did mention value_ptr on this site by name, not so long ago.

I originally named the class 'managed' but wanted something more inline with standard naming conventions (which generally end with '_ptr'), and the first thing that came to mind was the fact that it uses value-based semantics, hence value_ptr. But now that you mention it, you did in fact reference Dawson's class by name in a previous post, and so it's entirely possible that the name subconciously stuck to my mind and resurfaced later during the renaming stage.

At any rate, his class does work much the same, except that it doesn't interact with pointers directly (using references, instead), nor can it contain a null pointer, which almost qualifies it as a class similar to, but not exactly equivalent to, a smart-pointer (maybe quasi-smart-pointer). Overall, though, it appears to be a very well-designed library.

Originally Posted by CornedBee

In Boost, such a thing usually goes by the name clone_ptr. A search on the developer mailing list reveals several threads on classes of this type, and there even appears to be an implementation in the vault. I didn't take the time to look at specific objections, though. There were some efficiency concerns should such a pointer be stored in standard containers (especially vector), but the same problem exists for std::string and many others.

Interesting. Yep, it almost seemed impossible that something like this wouldn't have been suggested already. I'll definitely check that out. Thanks.