value_ptr - a C++11 header-only, deep-copying smart pointer that preserves value semantics for polymorphic...
up vote
13
down vote
favorite
My previous two iterations were here and here. I've since finalized the concept as described in the title, and would appreciate any feedback on the new solution located on GitHub.
Introduction:
value_ptr
is a C++11 header only, deep-copying smart pointer that preserves value semantics for both polymorphic and undefined types. value_ptr
aims to address the following issues by reducing/eliminating the boilerplate needed to facilitate value semantics: The polymorphic copy problem, and the undefined type problem.
The polymorphic copy problem. Given a class hierarchy, preserve value semantics while preventing object slicing.
Example:
Without value_ptr
:
struct Base { virtual Base* clone() const { return new Base(*this); } };
struct Derived : Base { Base* clone() const { return new Derived(*this); };
struct MyAwesomeClass {
std::unique_ptr<Base> foo;
};
int main() {
MyAwesomeClass a{};
// lets make a copy of a
auto b = a; // ERROR. Now we have to do a bunch of boilerplate to clone 'foo', etc. Boo!
}
With value_ptr
:
#include "value_ptr.hpp"
struct Base { virtual Base* clone() const { return new Base(*this); } };
struct Derived : Base { Base* clone() const { return new Derived(*this); };
struct MyAwesomeClass {
smart_ptr::value_ptr<Base> foo;
};
int main() {
MyAwesomeClass a{};
// lets make a copy of a
auto b = a; // no boilerplate, no slicing, no problem. yay!
}
The undefined type problem. Given a declared-but-not-yet defined type (which may or may not be polymorphic), preserve value semantics and prevent object slicing.
This problem is often seen in the PIMPL idiom and often associated with forward declared classes.
Example:
Without value_ptr
:
struct U; // undefined type, defined in another file somewhere that we can't/don't want to include
class MyAwesomeClass {
std::unique_ptr<U> u; // unique_ptr doesn't really fit, but we don't want to manage a raw pointer either.
};
MyAwesomeClass a{};
auto b = a; // ERROR C2280!
With value_ptr
:
struct U; // undefined type
class MyAwesomeClass {
smart_ptr::value_ptr<U> u;
};
MyAwesomeClass a{};
auto b = a; // no problem!
Features:
- Header only, single file, cross platform, no dependencies outside the STL
- Compatible interface/convertible to
std::unique_ptr
- Space efficient:
- Utilizes empty base optimization to minimize memory footprint
- defined types:
sizeof( value_ptr ) == sizeof(T*) == sizeof(std::unique_ptr)
- Undefined types:
sizeof( value_ptr ) == sizeof(T*)
+ two function pointers
- Polymorphic copying:
- Automatically detects/utilizes
clone()
member function
- Automatically detects/utilizes
- Static assertion prevents object slicing if a user-defined copier not provided or clone member not found
- Support for stateful and stateless deleters and copiers, via functors or lambdas
- Unit tested, valgrind clean
The code:
// Distributed under the Boost Software License, Version 1.0.
// (See http://www.boost.org/LICENSE_1_0.txt)
#ifndef _SMART_PTR_VALUE_PTR_
#define _SMART_PTR_VALUE_PTR_
#include <memory> // unique_ptr
#include <functional> // less_equal
#include <cassert> // assert
#if defined( _MSC_VER) // todo: check constexpr/delegating ctor issue in vs17. issue persists in vs15 update 3 despite ms closed bug as fixed, or i'm doing something wrong
#define VALUE_PTR_CONSTEXPR
#else
#define VALUE_PTR_CONSTEXPR constexpr
#endif
namespace smart_ptr {
namespace detail {
// void_t for c++11
// from http://en.cppreference.com/w/cpp/types/void_t
template<typename... Ts> struct make_void { typedef void type; };
template<typename... Ts> using void_t = typename make_void<Ts...>::type;
// is_defined<T>, from https://stackoverflow.com/a/39816909
template <class, class = void> struct is_defined : std::false_type { };
template <class T> struct is_defined<
T
, typename std::enable_if<std::is_object<T>::value && !std::is_pointer<T>::value && ( sizeof( T ) > 0 )>::type
>
: std::true_type{}
;
// Class function/type detection
// https://stackoverflow.com/a/30848101
// Primary template handles all types not supporting the operation.
template <typename, template <typename> class, typename = void_t<>>
struct detect : std::false_type {};
// Specialization recognizes/validates only types supporting the archetype.
template <typename T, template <typename> class Op>
struct detect<T, Op, void_t<Op<T>>> : std::true_type {};
// clone function
template <typename T> using fn_clone_t = decltype( std::declval<T>().clone() );
// has_clone
template <typename T> using has_clone = detect<T, fn_clone_t>;
// Returns flag if test passes (false==slicing is probable)
// T==base pointer, U==derived/supplied pointer
template <typename T, typename U, bool IsDefaultCopier>
struct slice_test : std::conditional<
std::is_same<T, U>::value // if U==T, no need to check for slicing
|| std::is_same<std::nullptr_t, U>::value // nullptr is fine
|| !IsDefaultCopier // user provided cloner, assume they're handling it
|| has_clone<typename std::remove_pointer<U>::type>::value // using default cloner, clone method must exist in U
, std::true_type
, std::false_type
>::type {};
// op_wrapper wraps Op::operator() and dispatches to observer fn
// observer fn then calls op_wrapper.op() to invoke Op::operator()
// this redirection is needed to call the actual operation in a context when T is actually defined
template <typename T, typename Op, typename R, typename ObserverFnSig>
struct op_wrapper : public Op {
using this_type = op_wrapper<T, Op, R, ObserverFnSig>;
using return_type = R;
// observer function to call
ObserverFnSig observer_fn;
template <typename Op_, typename Fn>
constexpr op_wrapper( Op_&& op, Fn&& obs )
: Op( std::forward<Op_>( op ) )
, observer_fn( std::forward<Fn>( obs ) )
{}
// invoked for event
template <typename... Args>
return_type operator()( Args&&... args ) const {
assert( this->observer_fn != nullptr );
// here we want to notify observer of event, with reference to this as first parameter
return this->observer_fn( (const void*)this, std::forward<Args>( args )... );
}
// call to actual operation (Op::operator()), invoked by observer
template <typename... Args>
return_type op( Args&&... args ) const {
return Op::operator()( std::forward<Args>(args)... );
}
}; // op_wrapper
// ptr_data
template <typename T, typename Deleter, typename Copier>
struct
#ifdef _MSC_VER
// https://blogs.msdn.microsoft.com/vcblog/2016/03/30/optimizing-the-layout-of-empty-base-classes-in-vs2015-update-2-3/
__declspec( empty_bases ) // requires vs2015 update 2
#endif
ptr_data
: public std::unique_ptr<T, Deleter>
, public Copier
{
using copier_type = Copier;
using base_type_uptr = std::unique_ptr<T, Deleter>;
using deleter_type = Deleter;
ptr_data() = default;
template <typename Dx, typename Cx>
constexpr ptr_data( T* px, Dx&& dx, Cx&& cx ) noexcept
: base_type_uptr( px, std::forward<Dx>(dx) )
, copier_type( std::forward<Cx>(cx) )
{}
copier_type& get_copier() { return static_cast<copier_type&>( *this ); }
const copier_type& get_copier() const { return static_cast<const copier_type&>( *this ); }
ptr_data clone() const {
return{ this->get_copier()( this->get() ), this->get_deleter(), this->get_copier() };
}
ptr_data( ptr_data&& ) = default;
ptr_data& operator=( ptr_data&& ) = default;
ptr_data( const ptr_data& that )
: ptr_data( that.clone() )
{}
ptr_data& operator=( const ptr_data& that ) {
if ( this == &that )
return *this;
*this = that.clone();
return *this;
}
}; // ptr_data
// ptr_base: base class for defined types
template <typename T, typename Deleter, typename Copier>
struct ptr_base {
using _data_type = ptr_data<T, Deleter, Copier>;
using _pointer = typename _data_type::pointer;
_data_type _data;
using pointer = _pointer;
template <typename Px, typename Dx, typename Cx>
constexpr ptr_base( Px&& px, Dx&& dx, Cx&& cx )
: _data(
std::forward<Px>( px )
, std::forward<Dx>( dx )
, std::forward<Cx>(cx)
)
{}
// conversion to unique_ptr
const typename _data_type::base_type_uptr& ptr() const {
return this->_data;
}
// conversion to unique_ptr
typename _data_type::base_type_uptr& ptr() {
return this->_data;
}
// conversion to unique_ptr
operator typename _data_type::base_type_uptr const&() const {
return this->_data;
}
// conversion to unique_ptr
operator typename _data_type::base_type_uptr& () {
return this->_data;
}
}; // ptr_base
// ptr_base_undefined: intermediate base class for undefined types
template <typename T, typename DeleteOp, typename CopyOp
, typename Deleter = op_wrapper<T, DeleteOp, void, void( *)( const void*, T* )>
, typename Copier = op_wrapper<T, CopyOp, T*, T*(*)(const void*, const T*)>
>
struct ptr_base_undefined
: ptr_base<T, Deleter, Copier> {
using base_type = ptr_base<T,Deleter,Copier>;
using pointer = typename base_type::pointer;
// default construct for undefined type
template <typename Dx, typename Cx>
constexpr ptr_base_undefined( std::nullptr_t, Dx&& dx, Cx&& cx )
: base_type(
nullptr
, Deleter( std::forward<Dx>( dx ), ( const void*, T* ptr ) { assert( ptr == nullptr ); } )
, Copier( std::forward<Cx>( cx ), ( const void* op, const T* ptr ) -> T* { assert( ptr == nullptr ); return nullptr; } )
)
{}
template <typename Dx, typename Cx>
constexpr ptr_base_undefined( pointer px, Dx&& dx, Cx&& cx )
: base_type(
px
, Deleter( std::forward<Dx>( dx ), ( const void* op, T* ptr ) {
if ( ptr )
static_cast<const Deleter*>( op )->op( ptr );
}
)
, Copier( std::forward<Cx>( cx ), ( const void* op, const T* ptr ) -> T* {
if ( !ptr )
return nullptr;
return static_cast<const Copier*>( op )->op( ptr );
}
)
)
{}
}; // ptr_base_undefined
} // detail
template <typename T>
struct default_copy {
// copy operator
T *operator()( const T* what ) const {
if ( !what )
return nullptr;
// tag dispatch on has_clone
return this->operator()( what
, typename std::conditional<detail::has_clone<T>::value, _clone, _copy>::type()
);
} // operator()
private:
struct _clone {};
struct _copy {};
T* operator()( const T* what, _clone ) const {
return what->clone();
}
T* operator()( const T* what, _copy ) const {
return new T( *what );
} // _copy
}; // default_copy
template <typename T
, typename Deleter = std::default_delete<T>
, typename Copier = default_copy<T>
, typename Base =
typename std::conditional<detail::is_defined<T>::value,
detail::ptr_base<T, Deleter, Copier>
, detail::ptr_base_undefined<T, Deleter, Copier>
>::type
>
struct value_ptr
: Base
{
using base_type = Base;
using element_type = T;
using pointer = typename base_type::pointer;
using const_pointer = const pointer;
using reference = typename std::add_lvalue_reference<element_type>::type;
using const_reference = const reference;
using deleter_type = Deleter;
using copier_type = Copier;
// construct with pointer, deleter, copier
template <typename Px>
constexpr value_ptr( Px px, deleter_type dx, copier_type cx )
: base_type( px
, std::move( dx )
, std::move( cx )
)
{
static_assert(
detail::slice_test<pointer, Px, std::is_same<default_copy<T>, copier_type>::value>::value
, "value_ptr; clone() method not detected and not using custom copier; slicing may occur"
);
}
// construct with pointer, deleter
template <typename Px>
VALUE_PTR_CONSTEXPR value_ptr( Px px, deleter_type dx ) // constexpr here yields c2476 on msvc15
: value_ptr( px, std::move(dx), copier_type() )
{}
// construct with pointer
template <typename Px>
VALUE_PTR_CONSTEXPR value_ptr( Px px ) // constexpr here yields c2476 on msvc15
: value_ptr( px, deleter_type(), copier_type() )
{}
// std::nullptr_t, default ctor
explicit VALUE_PTR_CONSTEXPR value_ptr( std::nullptr_t = nullptr ) // constexpr here yields c2476 on msvc15
: value_ptr( nullptr, deleter_type(), copier_type() )
{}
// get pointer
pointer get() { return this->_data.get(); }
// get const pointer
const_pointer get() const { return this->_data.get(); }
// reset pointer
template <typename Px = std::nullptr_t>
void reset( Px px = nullptr ) {
static_assert(
detail::slice_test<pointer, Px, std::is_same<default_copy<T>, copier_type>::value>::value
, "value_ptr; clone() method not detected and not using custom copier; slicing may occur"
);
*this = value_ptr( px, this->get_deleter(), this->get_copier() );
}
// release pointer
pointer release() noexcept {
return this->_data.release();
} // release
// return flag if has pointer
explicit operator bool() const {
return this->get() != nullptr;
}
const_reference operator*() const { return *this->get(); }
reference operator*() { return *this->get(); }
const_pointer operator-> () const { return this->get(); }
pointer operator-> () { return this->get(); }
void swap( value_ptr& that ) { std::swap( this->_data, that._data ); }
deleter_type& get_deleter() { return this->_data.get_deleter(); }
const deleter_type& get_deleter() const { return this->_data.get_deleter(); }
copier_type& get_copier() { return this->_data.get_copier(); }
const copier_type& get_copier() const { return this->_data.get_copier(); }
};// value_ptr
// non-member swap
template <class T1, class D1, class C1, class T2, class D2, class C2> void swap( value_ptr<T1, D1, C1>& x, value_ptr<T2, D2, C2>& y ) { x.swap( y ); }
// non-member operators
template <class T1, class D1, class C1, class T2, class D2, class C2> bool operator == ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return x.get() == y.get(); }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator != ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return x.get() != y.get(); }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator < ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) {
using common_type = typename std::common_type<typename value_ptr<T1, D1, C1>::pointer, typename value_ptr<T2, D2, C2>::pointer>::type;
return std::less<common_type>()( x.get(), y.get() );
}
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator <= ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return !( y < x ); }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator >( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return y < x; }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator >= ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return !( x < y ); }
template <class T, class D, class C> bool operator == ( const value_ptr<T, D, C>& x, std::nullptr_t ) noexcept { return !x; }
template <class T, class D, class C> bool operator == ( std::nullptr_t, const value_ptr<T, D, C>& x ) noexcept { return !x; }
template <class T, class D, class C> bool operator != ( const value_ptr<T, D, C>& x, std::nullptr_t ) noexcept { return (bool)x; }
template <class T, class D, class C> bool operator != ( std::nullptr_t, const value_ptr<T, D, C>& x ) noexcept { return (bool)x; }
template <class T, class D, class C> bool operator < ( const value_ptr<T, D, C>& x, std::nullptr_t ) { return std::less<typename value_ptr<T, D, C>::pointer>()( x.get(), nullptr ); }
template <class T, class D, class C> bool operator<( std::nullptr_t, const value_ptr<T, D, C>& y ) { return std::less<typename value_ptr<T, D, C>::pointer>()( nullptr, y.get() ); }
template <class T, class D, class C> bool operator <= ( const value_ptr<T, D, C>& x, std::nullptr_t ) { return std::less_equal<typename value_ptr<T, D, C>::pointer>()( x.get(), nullptr ); }
template <class T, class D, class C> bool operator <= ( std::nullptr_t, const value_ptr<T, D, C>& y ) { return std::less_equal<typename value_ptr<T, D, C>::pointer>()( nullptr, y.get() ); }
template <class T, class D, class C> bool operator >( const value_ptr<T, D, C>& x, std::nullptr_t ) { return !( nullptr < x ); }
template <class T, class D, class C> bool operator > ( std::nullptr_t, const value_ptr<T, D, C>& y ) { return !( y < nullptr ); }
template <class T, class D, class C> bool operator >= ( const value_ptr<T, D, C>& x, std::nullptr_t ) { return !( x < nullptr ); }
template <class T, class D, class C> bool operator >= ( std::nullptr_t, const value_ptr<T, D, C>& y ) { return !( nullptr < y ); }
template <typename T, typename Deleter>
static inline auto make_value_ptr( T* ptr, Deleter&& dx ) -> value_ptr<T, Deleter> {
return value_ptr<T, Deleter>( ptr, std::forward<Deleter>( dx ) );
} // make_value_ptr
template <typename T, typename Deleter, typename Copier>
static inline auto make_value_ptr( T* ptr, Deleter&& dx, Copier&& cx ) -> value_ptr<T, Deleter, Copier> {
return value_ptr<T, Deleter, Copier>( ptr, std::forward<Deleter>( dx ), std::forward<Copier>( cx ) );
} // make_value_ptr
} // smart_ptr ns
#undef VALUE_PTR_CONSTEXPR
#endif // !_SMART_PTR_VALUE_PTR_
c++ c++11 library pointers
add a comment |
up vote
13
down vote
favorite
My previous two iterations were here and here. I've since finalized the concept as described in the title, and would appreciate any feedback on the new solution located on GitHub.
Introduction:
value_ptr
is a C++11 header only, deep-copying smart pointer that preserves value semantics for both polymorphic and undefined types. value_ptr
aims to address the following issues by reducing/eliminating the boilerplate needed to facilitate value semantics: The polymorphic copy problem, and the undefined type problem.
The polymorphic copy problem. Given a class hierarchy, preserve value semantics while preventing object slicing.
Example:
Without value_ptr
:
struct Base { virtual Base* clone() const { return new Base(*this); } };
struct Derived : Base { Base* clone() const { return new Derived(*this); };
struct MyAwesomeClass {
std::unique_ptr<Base> foo;
};
int main() {
MyAwesomeClass a{};
// lets make a copy of a
auto b = a; // ERROR. Now we have to do a bunch of boilerplate to clone 'foo', etc. Boo!
}
With value_ptr
:
#include "value_ptr.hpp"
struct Base { virtual Base* clone() const { return new Base(*this); } };
struct Derived : Base { Base* clone() const { return new Derived(*this); };
struct MyAwesomeClass {
smart_ptr::value_ptr<Base> foo;
};
int main() {
MyAwesomeClass a{};
// lets make a copy of a
auto b = a; // no boilerplate, no slicing, no problem. yay!
}
The undefined type problem. Given a declared-but-not-yet defined type (which may or may not be polymorphic), preserve value semantics and prevent object slicing.
This problem is often seen in the PIMPL idiom and often associated with forward declared classes.
Example:
Without value_ptr
:
struct U; // undefined type, defined in another file somewhere that we can't/don't want to include
class MyAwesomeClass {
std::unique_ptr<U> u; // unique_ptr doesn't really fit, but we don't want to manage a raw pointer either.
};
MyAwesomeClass a{};
auto b = a; // ERROR C2280!
With value_ptr
:
struct U; // undefined type
class MyAwesomeClass {
smart_ptr::value_ptr<U> u;
};
MyAwesomeClass a{};
auto b = a; // no problem!
Features:
- Header only, single file, cross platform, no dependencies outside the STL
- Compatible interface/convertible to
std::unique_ptr
- Space efficient:
- Utilizes empty base optimization to minimize memory footprint
- defined types:
sizeof( value_ptr ) == sizeof(T*) == sizeof(std::unique_ptr)
- Undefined types:
sizeof( value_ptr ) == sizeof(T*)
+ two function pointers
- Polymorphic copying:
- Automatically detects/utilizes
clone()
member function
- Automatically detects/utilizes
- Static assertion prevents object slicing if a user-defined copier not provided or clone member not found
- Support for stateful and stateless deleters and copiers, via functors or lambdas
- Unit tested, valgrind clean
The code:
// Distributed under the Boost Software License, Version 1.0.
// (See http://www.boost.org/LICENSE_1_0.txt)
#ifndef _SMART_PTR_VALUE_PTR_
#define _SMART_PTR_VALUE_PTR_
#include <memory> // unique_ptr
#include <functional> // less_equal
#include <cassert> // assert
#if defined( _MSC_VER) // todo: check constexpr/delegating ctor issue in vs17. issue persists in vs15 update 3 despite ms closed bug as fixed, or i'm doing something wrong
#define VALUE_PTR_CONSTEXPR
#else
#define VALUE_PTR_CONSTEXPR constexpr
#endif
namespace smart_ptr {
namespace detail {
// void_t for c++11
// from http://en.cppreference.com/w/cpp/types/void_t
template<typename... Ts> struct make_void { typedef void type; };
template<typename... Ts> using void_t = typename make_void<Ts...>::type;
// is_defined<T>, from https://stackoverflow.com/a/39816909
template <class, class = void> struct is_defined : std::false_type { };
template <class T> struct is_defined<
T
, typename std::enable_if<std::is_object<T>::value && !std::is_pointer<T>::value && ( sizeof( T ) > 0 )>::type
>
: std::true_type{}
;
// Class function/type detection
// https://stackoverflow.com/a/30848101
// Primary template handles all types not supporting the operation.
template <typename, template <typename> class, typename = void_t<>>
struct detect : std::false_type {};
// Specialization recognizes/validates only types supporting the archetype.
template <typename T, template <typename> class Op>
struct detect<T, Op, void_t<Op<T>>> : std::true_type {};
// clone function
template <typename T> using fn_clone_t = decltype( std::declval<T>().clone() );
// has_clone
template <typename T> using has_clone = detect<T, fn_clone_t>;
// Returns flag if test passes (false==slicing is probable)
// T==base pointer, U==derived/supplied pointer
template <typename T, typename U, bool IsDefaultCopier>
struct slice_test : std::conditional<
std::is_same<T, U>::value // if U==T, no need to check for slicing
|| std::is_same<std::nullptr_t, U>::value // nullptr is fine
|| !IsDefaultCopier // user provided cloner, assume they're handling it
|| has_clone<typename std::remove_pointer<U>::type>::value // using default cloner, clone method must exist in U
, std::true_type
, std::false_type
>::type {};
// op_wrapper wraps Op::operator() and dispatches to observer fn
// observer fn then calls op_wrapper.op() to invoke Op::operator()
// this redirection is needed to call the actual operation in a context when T is actually defined
template <typename T, typename Op, typename R, typename ObserverFnSig>
struct op_wrapper : public Op {
using this_type = op_wrapper<T, Op, R, ObserverFnSig>;
using return_type = R;
// observer function to call
ObserverFnSig observer_fn;
template <typename Op_, typename Fn>
constexpr op_wrapper( Op_&& op, Fn&& obs )
: Op( std::forward<Op_>( op ) )
, observer_fn( std::forward<Fn>( obs ) )
{}
// invoked for event
template <typename... Args>
return_type operator()( Args&&... args ) const {
assert( this->observer_fn != nullptr );
// here we want to notify observer of event, with reference to this as first parameter
return this->observer_fn( (const void*)this, std::forward<Args>( args )... );
}
// call to actual operation (Op::operator()), invoked by observer
template <typename... Args>
return_type op( Args&&... args ) const {
return Op::operator()( std::forward<Args>(args)... );
}
}; // op_wrapper
// ptr_data
template <typename T, typename Deleter, typename Copier>
struct
#ifdef _MSC_VER
// https://blogs.msdn.microsoft.com/vcblog/2016/03/30/optimizing-the-layout-of-empty-base-classes-in-vs2015-update-2-3/
__declspec( empty_bases ) // requires vs2015 update 2
#endif
ptr_data
: public std::unique_ptr<T, Deleter>
, public Copier
{
using copier_type = Copier;
using base_type_uptr = std::unique_ptr<T, Deleter>;
using deleter_type = Deleter;
ptr_data() = default;
template <typename Dx, typename Cx>
constexpr ptr_data( T* px, Dx&& dx, Cx&& cx ) noexcept
: base_type_uptr( px, std::forward<Dx>(dx) )
, copier_type( std::forward<Cx>(cx) )
{}
copier_type& get_copier() { return static_cast<copier_type&>( *this ); }
const copier_type& get_copier() const { return static_cast<const copier_type&>( *this ); }
ptr_data clone() const {
return{ this->get_copier()( this->get() ), this->get_deleter(), this->get_copier() };
}
ptr_data( ptr_data&& ) = default;
ptr_data& operator=( ptr_data&& ) = default;
ptr_data( const ptr_data& that )
: ptr_data( that.clone() )
{}
ptr_data& operator=( const ptr_data& that ) {
if ( this == &that )
return *this;
*this = that.clone();
return *this;
}
}; // ptr_data
// ptr_base: base class for defined types
template <typename T, typename Deleter, typename Copier>
struct ptr_base {
using _data_type = ptr_data<T, Deleter, Copier>;
using _pointer = typename _data_type::pointer;
_data_type _data;
using pointer = _pointer;
template <typename Px, typename Dx, typename Cx>
constexpr ptr_base( Px&& px, Dx&& dx, Cx&& cx )
: _data(
std::forward<Px>( px )
, std::forward<Dx>( dx )
, std::forward<Cx>(cx)
)
{}
// conversion to unique_ptr
const typename _data_type::base_type_uptr& ptr() const {
return this->_data;
}
// conversion to unique_ptr
typename _data_type::base_type_uptr& ptr() {
return this->_data;
}
// conversion to unique_ptr
operator typename _data_type::base_type_uptr const&() const {
return this->_data;
}
// conversion to unique_ptr
operator typename _data_type::base_type_uptr& () {
return this->_data;
}
}; // ptr_base
// ptr_base_undefined: intermediate base class for undefined types
template <typename T, typename DeleteOp, typename CopyOp
, typename Deleter = op_wrapper<T, DeleteOp, void, void( *)( const void*, T* )>
, typename Copier = op_wrapper<T, CopyOp, T*, T*(*)(const void*, const T*)>
>
struct ptr_base_undefined
: ptr_base<T, Deleter, Copier> {
using base_type = ptr_base<T,Deleter,Copier>;
using pointer = typename base_type::pointer;
// default construct for undefined type
template <typename Dx, typename Cx>
constexpr ptr_base_undefined( std::nullptr_t, Dx&& dx, Cx&& cx )
: base_type(
nullptr
, Deleter( std::forward<Dx>( dx ), ( const void*, T* ptr ) { assert( ptr == nullptr ); } )
, Copier( std::forward<Cx>( cx ), ( const void* op, const T* ptr ) -> T* { assert( ptr == nullptr ); return nullptr; } )
)
{}
template <typename Dx, typename Cx>
constexpr ptr_base_undefined( pointer px, Dx&& dx, Cx&& cx )
: base_type(
px
, Deleter( std::forward<Dx>( dx ), ( const void* op, T* ptr ) {
if ( ptr )
static_cast<const Deleter*>( op )->op( ptr );
}
)
, Copier( std::forward<Cx>( cx ), ( const void* op, const T* ptr ) -> T* {
if ( !ptr )
return nullptr;
return static_cast<const Copier*>( op )->op( ptr );
}
)
)
{}
}; // ptr_base_undefined
} // detail
template <typename T>
struct default_copy {
// copy operator
T *operator()( const T* what ) const {
if ( !what )
return nullptr;
// tag dispatch on has_clone
return this->operator()( what
, typename std::conditional<detail::has_clone<T>::value, _clone, _copy>::type()
);
} // operator()
private:
struct _clone {};
struct _copy {};
T* operator()( const T* what, _clone ) const {
return what->clone();
}
T* operator()( const T* what, _copy ) const {
return new T( *what );
} // _copy
}; // default_copy
template <typename T
, typename Deleter = std::default_delete<T>
, typename Copier = default_copy<T>
, typename Base =
typename std::conditional<detail::is_defined<T>::value,
detail::ptr_base<T, Deleter, Copier>
, detail::ptr_base_undefined<T, Deleter, Copier>
>::type
>
struct value_ptr
: Base
{
using base_type = Base;
using element_type = T;
using pointer = typename base_type::pointer;
using const_pointer = const pointer;
using reference = typename std::add_lvalue_reference<element_type>::type;
using const_reference = const reference;
using deleter_type = Deleter;
using copier_type = Copier;
// construct with pointer, deleter, copier
template <typename Px>
constexpr value_ptr( Px px, deleter_type dx, copier_type cx )
: base_type( px
, std::move( dx )
, std::move( cx )
)
{
static_assert(
detail::slice_test<pointer, Px, std::is_same<default_copy<T>, copier_type>::value>::value
, "value_ptr; clone() method not detected and not using custom copier; slicing may occur"
);
}
// construct with pointer, deleter
template <typename Px>
VALUE_PTR_CONSTEXPR value_ptr( Px px, deleter_type dx ) // constexpr here yields c2476 on msvc15
: value_ptr( px, std::move(dx), copier_type() )
{}
// construct with pointer
template <typename Px>
VALUE_PTR_CONSTEXPR value_ptr( Px px ) // constexpr here yields c2476 on msvc15
: value_ptr( px, deleter_type(), copier_type() )
{}
// std::nullptr_t, default ctor
explicit VALUE_PTR_CONSTEXPR value_ptr( std::nullptr_t = nullptr ) // constexpr here yields c2476 on msvc15
: value_ptr( nullptr, deleter_type(), copier_type() )
{}
// get pointer
pointer get() { return this->_data.get(); }
// get const pointer
const_pointer get() const { return this->_data.get(); }
// reset pointer
template <typename Px = std::nullptr_t>
void reset( Px px = nullptr ) {
static_assert(
detail::slice_test<pointer, Px, std::is_same<default_copy<T>, copier_type>::value>::value
, "value_ptr; clone() method not detected and not using custom copier; slicing may occur"
);
*this = value_ptr( px, this->get_deleter(), this->get_copier() );
}
// release pointer
pointer release() noexcept {
return this->_data.release();
} // release
// return flag if has pointer
explicit operator bool() const {
return this->get() != nullptr;
}
const_reference operator*() const { return *this->get(); }
reference operator*() { return *this->get(); }
const_pointer operator-> () const { return this->get(); }
pointer operator-> () { return this->get(); }
void swap( value_ptr& that ) { std::swap( this->_data, that._data ); }
deleter_type& get_deleter() { return this->_data.get_deleter(); }
const deleter_type& get_deleter() const { return this->_data.get_deleter(); }
copier_type& get_copier() { return this->_data.get_copier(); }
const copier_type& get_copier() const { return this->_data.get_copier(); }
};// value_ptr
// non-member swap
template <class T1, class D1, class C1, class T2, class D2, class C2> void swap( value_ptr<T1, D1, C1>& x, value_ptr<T2, D2, C2>& y ) { x.swap( y ); }
// non-member operators
template <class T1, class D1, class C1, class T2, class D2, class C2> bool operator == ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return x.get() == y.get(); }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator != ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return x.get() != y.get(); }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator < ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) {
using common_type = typename std::common_type<typename value_ptr<T1, D1, C1>::pointer, typename value_ptr<T2, D2, C2>::pointer>::type;
return std::less<common_type>()( x.get(), y.get() );
}
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator <= ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return !( y < x ); }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator >( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return y < x; }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator >= ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return !( x < y ); }
template <class T, class D, class C> bool operator == ( const value_ptr<T, D, C>& x, std::nullptr_t ) noexcept { return !x; }
template <class T, class D, class C> bool operator == ( std::nullptr_t, const value_ptr<T, D, C>& x ) noexcept { return !x; }
template <class T, class D, class C> bool operator != ( const value_ptr<T, D, C>& x, std::nullptr_t ) noexcept { return (bool)x; }
template <class T, class D, class C> bool operator != ( std::nullptr_t, const value_ptr<T, D, C>& x ) noexcept { return (bool)x; }
template <class T, class D, class C> bool operator < ( const value_ptr<T, D, C>& x, std::nullptr_t ) { return std::less<typename value_ptr<T, D, C>::pointer>()( x.get(), nullptr ); }
template <class T, class D, class C> bool operator<( std::nullptr_t, const value_ptr<T, D, C>& y ) { return std::less<typename value_ptr<T, D, C>::pointer>()( nullptr, y.get() ); }
template <class T, class D, class C> bool operator <= ( const value_ptr<T, D, C>& x, std::nullptr_t ) { return std::less_equal<typename value_ptr<T, D, C>::pointer>()( x.get(), nullptr ); }
template <class T, class D, class C> bool operator <= ( std::nullptr_t, const value_ptr<T, D, C>& y ) { return std::less_equal<typename value_ptr<T, D, C>::pointer>()( nullptr, y.get() ); }
template <class T, class D, class C> bool operator >( const value_ptr<T, D, C>& x, std::nullptr_t ) { return !( nullptr < x ); }
template <class T, class D, class C> bool operator > ( std::nullptr_t, const value_ptr<T, D, C>& y ) { return !( y < nullptr ); }
template <class T, class D, class C> bool operator >= ( const value_ptr<T, D, C>& x, std::nullptr_t ) { return !( x < nullptr ); }
template <class T, class D, class C> bool operator >= ( std::nullptr_t, const value_ptr<T, D, C>& y ) { return !( nullptr < y ); }
template <typename T, typename Deleter>
static inline auto make_value_ptr( T* ptr, Deleter&& dx ) -> value_ptr<T, Deleter> {
return value_ptr<T, Deleter>( ptr, std::forward<Deleter>( dx ) );
} // make_value_ptr
template <typename T, typename Deleter, typename Copier>
static inline auto make_value_ptr( T* ptr, Deleter&& dx, Copier&& cx ) -> value_ptr<T, Deleter, Copier> {
return value_ptr<T, Deleter, Copier>( ptr, std::forward<Deleter>( dx ), std::forward<Copier>( cx ) );
} // make_value_ptr
} // smart_ptr ns
#undef VALUE_PTR_CONSTEXPR
#endif // !_SMART_PTR_VALUE_PTR_
c++ c++11 library pointers
1
Shame nobody seems to have time to review this, it’s a very nice submission. Unfortunately I lack the time at the moment; just one thing I noticed; the identifier_SMART_PTR_VALUE_PTR_
is reserved for the implementation, and is illegal in client code; the rule is: everything in the global namespace starting with__
(double underscore) or_
(single underscore) + uppercase letter is reserved.
– Konrad Rudolph
Jul 14 '17 at 14:33
@KonradRudolph - thank you! I didn't know that about the leading underscore
– Tom
Jul 14 '17 at 16:33
Wow. I had never heard of__declspec( empty_bases )
before now. That's something I'm gonna have to use.
– Justin
Aug 2 '17 at 21:10
1
@Justin, there is standard solution coming, IIRC it is called[[no_unique_address]]
.
– Incomputable
Oct 30 at 5:33
add a comment |
up vote
13
down vote
favorite
up vote
13
down vote
favorite
My previous two iterations were here and here. I've since finalized the concept as described in the title, and would appreciate any feedback on the new solution located on GitHub.
Introduction:
value_ptr
is a C++11 header only, deep-copying smart pointer that preserves value semantics for both polymorphic and undefined types. value_ptr
aims to address the following issues by reducing/eliminating the boilerplate needed to facilitate value semantics: The polymorphic copy problem, and the undefined type problem.
The polymorphic copy problem. Given a class hierarchy, preserve value semantics while preventing object slicing.
Example:
Without value_ptr
:
struct Base { virtual Base* clone() const { return new Base(*this); } };
struct Derived : Base { Base* clone() const { return new Derived(*this); };
struct MyAwesomeClass {
std::unique_ptr<Base> foo;
};
int main() {
MyAwesomeClass a{};
// lets make a copy of a
auto b = a; // ERROR. Now we have to do a bunch of boilerplate to clone 'foo', etc. Boo!
}
With value_ptr
:
#include "value_ptr.hpp"
struct Base { virtual Base* clone() const { return new Base(*this); } };
struct Derived : Base { Base* clone() const { return new Derived(*this); };
struct MyAwesomeClass {
smart_ptr::value_ptr<Base> foo;
};
int main() {
MyAwesomeClass a{};
// lets make a copy of a
auto b = a; // no boilerplate, no slicing, no problem. yay!
}
The undefined type problem. Given a declared-but-not-yet defined type (which may or may not be polymorphic), preserve value semantics and prevent object slicing.
This problem is often seen in the PIMPL idiom and often associated with forward declared classes.
Example:
Without value_ptr
:
struct U; // undefined type, defined in another file somewhere that we can't/don't want to include
class MyAwesomeClass {
std::unique_ptr<U> u; // unique_ptr doesn't really fit, but we don't want to manage a raw pointer either.
};
MyAwesomeClass a{};
auto b = a; // ERROR C2280!
With value_ptr
:
struct U; // undefined type
class MyAwesomeClass {
smart_ptr::value_ptr<U> u;
};
MyAwesomeClass a{};
auto b = a; // no problem!
Features:
- Header only, single file, cross platform, no dependencies outside the STL
- Compatible interface/convertible to
std::unique_ptr
- Space efficient:
- Utilizes empty base optimization to minimize memory footprint
- defined types:
sizeof( value_ptr ) == sizeof(T*) == sizeof(std::unique_ptr)
- Undefined types:
sizeof( value_ptr ) == sizeof(T*)
+ two function pointers
- Polymorphic copying:
- Automatically detects/utilizes
clone()
member function
- Automatically detects/utilizes
- Static assertion prevents object slicing if a user-defined copier not provided or clone member not found
- Support for stateful and stateless deleters and copiers, via functors or lambdas
- Unit tested, valgrind clean
The code:
// Distributed under the Boost Software License, Version 1.0.
// (See http://www.boost.org/LICENSE_1_0.txt)
#ifndef _SMART_PTR_VALUE_PTR_
#define _SMART_PTR_VALUE_PTR_
#include <memory> // unique_ptr
#include <functional> // less_equal
#include <cassert> // assert
#if defined( _MSC_VER) // todo: check constexpr/delegating ctor issue in vs17. issue persists in vs15 update 3 despite ms closed bug as fixed, or i'm doing something wrong
#define VALUE_PTR_CONSTEXPR
#else
#define VALUE_PTR_CONSTEXPR constexpr
#endif
namespace smart_ptr {
namespace detail {
// void_t for c++11
// from http://en.cppreference.com/w/cpp/types/void_t
template<typename... Ts> struct make_void { typedef void type; };
template<typename... Ts> using void_t = typename make_void<Ts...>::type;
// is_defined<T>, from https://stackoverflow.com/a/39816909
template <class, class = void> struct is_defined : std::false_type { };
template <class T> struct is_defined<
T
, typename std::enable_if<std::is_object<T>::value && !std::is_pointer<T>::value && ( sizeof( T ) > 0 )>::type
>
: std::true_type{}
;
// Class function/type detection
// https://stackoverflow.com/a/30848101
// Primary template handles all types not supporting the operation.
template <typename, template <typename> class, typename = void_t<>>
struct detect : std::false_type {};
// Specialization recognizes/validates only types supporting the archetype.
template <typename T, template <typename> class Op>
struct detect<T, Op, void_t<Op<T>>> : std::true_type {};
// clone function
template <typename T> using fn_clone_t = decltype( std::declval<T>().clone() );
// has_clone
template <typename T> using has_clone = detect<T, fn_clone_t>;
// Returns flag if test passes (false==slicing is probable)
// T==base pointer, U==derived/supplied pointer
template <typename T, typename U, bool IsDefaultCopier>
struct slice_test : std::conditional<
std::is_same<T, U>::value // if U==T, no need to check for slicing
|| std::is_same<std::nullptr_t, U>::value // nullptr is fine
|| !IsDefaultCopier // user provided cloner, assume they're handling it
|| has_clone<typename std::remove_pointer<U>::type>::value // using default cloner, clone method must exist in U
, std::true_type
, std::false_type
>::type {};
// op_wrapper wraps Op::operator() and dispatches to observer fn
// observer fn then calls op_wrapper.op() to invoke Op::operator()
// this redirection is needed to call the actual operation in a context when T is actually defined
template <typename T, typename Op, typename R, typename ObserverFnSig>
struct op_wrapper : public Op {
using this_type = op_wrapper<T, Op, R, ObserverFnSig>;
using return_type = R;
// observer function to call
ObserverFnSig observer_fn;
template <typename Op_, typename Fn>
constexpr op_wrapper( Op_&& op, Fn&& obs )
: Op( std::forward<Op_>( op ) )
, observer_fn( std::forward<Fn>( obs ) )
{}
// invoked for event
template <typename... Args>
return_type operator()( Args&&... args ) const {
assert( this->observer_fn != nullptr );
// here we want to notify observer of event, with reference to this as first parameter
return this->observer_fn( (const void*)this, std::forward<Args>( args )... );
}
// call to actual operation (Op::operator()), invoked by observer
template <typename... Args>
return_type op( Args&&... args ) const {
return Op::operator()( std::forward<Args>(args)... );
}
}; // op_wrapper
// ptr_data
template <typename T, typename Deleter, typename Copier>
struct
#ifdef _MSC_VER
// https://blogs.msdn.microsoft.com/vcblog/2016/03/30/optimizing-the-layout-of-empty-base-classes-in-vs2015-update-2-3/
__declspec( empty_bases ) // requires vs2015 update 2
#endif
ptr_data
: public std::unique_ptr<T, Deleter>
, public Copier
{
using copier_type = Copier;
using base_type_uptr = std::unique_ptr<T, Deleter>;
using deleter_type = Deleter;
ptr_data() = default;
template <typename Dx, typename Cx>
constexpr ptr_data( T* px, Dx&& dx, Cx&& cx ) noexcept
: base_type_uptr( px, std::forward<Dx>(dx) )
, copier_type( std::forward<Cx>(cx) )
{}
copier_type& get_copier() { return static_cast<copier_type&>( *this ); }
const copier_type& get_copier() const { return static_cast<const copier_type&>( *this ); }
ptr_data clone() const {
return{ this->get_copier()( this->get() ), this->get_deleter(), this->get_copier() };
}
ptr_data( ptr_data&& ) = default;
ptr_data& operator=( ptr_data&& ) = default;
ptr_data( const ptr_data& that )
: ptr_data( that.clone() )
{}
ptr_data& operator=( const ptr_data& that ) {
if ( this == &that )
return *this;
*this = that.clone();
return *this;
}
}; // ptr_data
// ptr_base: base class for defined types
template <typename T, typename Deleter, typename Copier>
struct ptr_base {
using _data_type = ptr_data<T, Deleter, Copier>;
using _pointer = typename _data_type::pointer;
_data_type _data;
using pointer = _pointer;
template <typename Px, typename Dx, typename Cx>
constexpr ptr_base( Px&& px, Dx&& dx, Cx&& cx )
: _data(
std::forward<Px>( px )
, std::forward<Dx>( dx )
, std::forward<Cx>(cx)
)
{}
// conversion to unique_ptr
const typename _data_type::base_type_uptr& ptr() const {
return this->_data;
}
// conversion to unique_ptr
typename _data_type::base_type_uptr& ptr() {
return this->_data;
}
// conversion to unique_ptr
operator typename _data_type::base_type_uptr const&() const {
return this->_data;
}
// conversion to unique_ptr
operator typename _data_type::base_type_uptr& () {
return this->_data;
}
}; // ptr_base
// ptr_base_undefined: intermediate base class for undefined types
template <typename T, typename DeleteOp, typename CopyOp
, typename Deleter = op_wrapper<T, DeleteOp, void, void( *)( const void*, T* )>
, typename Copier = op_wrapper<T, CopyOp, T*, T*(*)(const void*, const T*)>
>
struct ptr_base_undefined
: ptr_base<T, Deleter, Copier> {
using base_type = ptr_base<T,Deleter,Copier>;
using pointer = typename base_type::pointer;
// default construct for undefined type
template <typename Dx, typename Cx>
constexpr ptr_base_undefined( std::nullptr_t, Dx&& dx, Cx&& cx )
: base_type(
nullptr
, Deleter( std::forward<Dx>( dx ), ( const void*, T* ptr ) { assert( ptr == nullptr ); } )
, Copier( std::forward<Cx>( cx ), ( const void* op, const T* ptr ) -> T* { assert( ptr == nullptr ); return nullptr; } )
)
{}
template <typename Dx, typename Cx>
constexpr ptr_base_undefined( pointer px, Dx&& dx, Cx&& cx )
: base_type(
px
, Deleter( std::forward<Dx>( dx ), ( const void* op, T* ptr ) {
if ( ptr )
static_cast<const Deleter*>( op )->op( ptr );
}
)
, Copier( std::forward<Cx>( cx ), ( const void* op, const T* ptr ) -> T* {
if ( !ptr )
return nullptr;
return static_cast<const Copier*>( op )->op( ptr );
}
)
)
{}
}; // ptr_base_undefined
} // detail
template <typename T>
struct default_copy {
// copy operator
T *operator()( const T* what ) const {
if ( !what )
return nullptr;
// tag dispatch on has_clone
return this->operator()( what
, typename std::conditional<detail::has_clone<T>::value, _clone, _copy>::type()
);
} // operator()
private:
struct _clone {};
struct _copy {};
T* operator()( const T* what, _clone ) const {
return what->clone();
}
T* operator()( const T* what, _copy ) const {
return new T( *what );
} // _copy
}; // default_copy
template <typename T
, typename Deleter = std::default_delete<T>
, typename Copier = default_copy<T>
, typename Base =
typename std::conditional<detail::is_defined<T>::value,
detail::ptr_base<T, Deleter, Copier>
, detail::ptr_base_undefined<T, Deleter, Copier>
>::type
>
struct value_ptr
: Base
{
using base_type = Base;
using element_type = T;
using pointer = typename base_type::pointer;
using const_pointer = const pointer;
using reference = typename std::add_lvalue_reference<element_type>::type;
using const_reference = const reference;
using deleter_type = Deleter;
using copier_type = Copier;
// construct with pointer, deleter, copier
template <typename Px>
constexpr value_ptr( Px px, deleter_type dx, copier_type cx )
: base_type( px
, std::move( dx )
, std::move( cx )
)
{
static_assert(
detail::slice_test<pointer, Px, std::is_same<default_copy<T>, copier_type>::value>::value
, "value_ptr; clone() method not detected and not using custom copier; slicing may occur"
);
}
// construct with pointer, deleter
template <typename Px>
VALUE_PTR_CONSTEXPR value_ptr( Px px, deleter_type dx ) // constexpr here yields c2476 on msvc15
: value_ptr( px, std::move(dx), copier_type() )
{}
// construct with pointer
template <typename Px>
VALUE_PTR_CONSTEXPR value_ptr( Px px ) // constexpr here yields c2476 on msvc15
: value_ptr( px, deleter_type(), copier_type() )
{}
// std::nullptr_t, default ctor
explicit VALUE_PTR_CONSTEXPR value_ptr( std::nullptr_t = nullptr ) // constexpr here yields c2476 on msvc15
: value_ptr( nullptr, deleter_type(), copier_type() )
{}
// get pointer
pointer get() { return this->_data.get(); }
// get const pointer
const_pointer get() const { return this->_data.get(); }
// reset pointer
template <typename Px = std::nullptr_t>
void reset( Px px = nullptr ) {
static_assert(
detail::slice_test<pointer, Px, std::is_same<default_copy<T>, copier_type>::value>::value
, "value_ptr; clone() method not detected and not using custom copier; slicing may occur"
);
*this = value_ptr( px, this->get_deleter(), this->get_copier() );
}
// release pointer
pointer release() noexcept {
return this->_data.release();
} // release
// return flag if has pointer
explicit operator bool() const {
return this->get() != nullptr;
}
const_reference operator*() const { return *this->get(); }
reference operator*() { return *this->get(); }
const_pointer operator-> () const { return this->get(); }
pointer operator-> () { return this->get(); }
void swap( value_ptr& that ) { std::swap( this->_data, that._data ); }
deleter_type& get_deleter() { return this->_data.get_deleter(); }
const deleter_type& get_deleter() const { return this->_data.get_deleter(); }
copier_type& get_copier() { return this->_data.get_copier(); }
const copier_type& get_copier() const { return this->_data.get_copier(); }
};// value_ptr
// non-member swap
template <class T1, class D1, class C1, class T2, class D2, class C2> void swap( value_ptr<T1, D1, C1>& x, value_ptr<T2, D2, C2>& y ) { x.swap( y ); }
// non-member operators
template <class T1, class D1, class C1, class T2, class D2, class C2> bool operator == ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return x.get() == y.get(); }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator != ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return x.get() != y.get(); }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator < ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) {
using common_type = typename std::common_type<typename value_ptr<T1, D1, C1>::pointer, typename value_ptr<T2, D2, C2>::pointer>::type;
return std::less<common_type>()( x.get(), y.get() );
}
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator <= ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return !( y < x ); }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator >( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return y < x; }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator >= ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return !( x < y ); }
template <class T, class D, class C> bool operator == ( const value_ptr<T, D, C>& x, std::nullptr_t ) noexcept { return !x; }
template <class T, class D, class C> bool operator == ( std::nullptr_t, const value_ptr<T, D, C>& x ) noexcept { return !x; }
template <class T, class D, class C> bool operator != ( const value_ptr<T, D, C>& x, std::nullptr_t ) noexcept { return (bool)x; }
template <class T, class D, class C> bool operator != ( std::nullptr_t, const value_ptr<T, D, C>& x ) noexcept { return (bool)x; }
template <class T, class D, class C> bool operator < ( const value_ptr<T, D, C>& x, std::nullptr_t ) { return std::less<typename value_ptr<T, D, C>::pointer>()( x.get(), nullptr ); }
template <class T, class D, class C> bool operator<( std::nullptr_t, const value_ptr<T, D, C>& y ) { return std::less<typename value_ptr<T, D, C>::pointer>()( nullptr, y.get() ); }
template <class T, class D, class C> bool operator <= ( const value_ptr<T, D, C>& x, std::nullptr_t ) { return std::less_equal<typename value_ptr<T, D, C>::pointer>()( x.get(), nullptr ); }
template <class T, class D, class C> bool operator <= ( std::nullptr_t, const value_ptr<T, D, C>& y ) { return std::less_equal<typename value_ptr<T, D, C>::pointer>()( nullptr, y.get() ); }
template <class T, class D, class C> bool operator >( const value_ptr<T, D, C>& x, std::nullptr_t ) { return !( nullptr < x ); }
template <class T, class D, class C> bool operator > ( std::nullptr_t, const value_ptr<T, D, C>& y ) { return !( y < nullptr ); }
template <class T, class D, class C> bool operator >= ( const value_ptr<T, D, C>& x, std::nullptr_t ) { return !( x < nullptr ); }
template <class T, class D, class C> bool operator >= ( std::nullptr_t, const value_ptr<T, D, C>& y ) { return !( nullptr < y ); }
template <typename T, typename Deleter>
static inline auto make_value_ptr( T* ptr, Deleter&& dx ) -> value_ptr<T, Deleter> {
return value_ptr<T, Deleter>( ptr, std::forward<Deleter>( dx ) );
} // make_value_ptr
template <typename T, typename Deleter, typename Copier>
static inline auto make_value_ptr( T* ptr, Deleter&& dx, Copier&& cx ) -> value_ptr<T, Deleter, Copier> {
return value_ptr<T, Deleter, Copier>( ptr, std::forward<Deleter>( dx ), std::forward<Copier>( cx ) );
} // make_value_ptr
} // smart_ptr ns
#undef VALUE_PTR_CONSTEXPR
#endif // !_SMART_PTR_VALUE_PTR_
c++ c++11 library pointers
My previous two iterations were here and here. I've since finalized the concept as described in the title, and would appreciate any feedback on the new solution located on GitHub.
Introduction:
value_ptr
is a C++11 header only, deep-copying smart pointer that preserves value semantics for both polymorphic and undefined types. value_ptr
aims to address the following issues by reducing/eliminating the boilerplate needed to facilitate value semantics: The polymorphic copy problem, and the undefined type problem.
The polymorphic copy problem. Given a class hierarchy, preserve value semantics while preventing object slicing.
Example:
Without value_ptr
:
struct Base { virtual Base* clone() const { return new Base(*this); } };
struct Derived : Base { Base* clone() const { return new Derived(*this); };
struct MyAwesomeClass {
std::unique_ptr<Base> foo;
};
int main() {
MyAwesomeClass a{};
// lets make a copy of a
auto b = a; // ERROR. Now we have to do a bunch of boilerplate to clone 'foo', etc. Boo!
}
With value_ptr
:
#include "value_ptr.hpp"
struct Base { virtual Base* clone() const { return new Base(*this); } };
struct Derived : Base { Base* clone() const { return new Derived(*this); };
struct MyAwesomeClass {
smart_ptr::value_ptr<Base> foo;
};
int main() {
MyAwesomeClass a{};
// lets make a copy of a
auto b = a; // no boilerplate, no slicing, no problem. yay!
}
The undefined type problem. Given a declared-but-not-yet defined type (which may or may not be polymorphic), preserve value semantics and prevent object slicing.
This problem is often seen in the PIMPL idiom and often associated with forward declared classes.
Example:
Without value_ptr
:
struct U; // undefined type, defined in another file somewhere that we can't/don't want to include
class MyAwesomeClass {
std::unique_ptr<U> u; // unique_ptr doesn't really fit, but we don't want to manage a raw pointer either.
};
MyAwesomeClass a{};
auto b = a; // ERROR C2280!
With value_ptr
:
struct U; // undefined type
class MyAwesomeClass {
smart_ptr::value_ptr<U> u;
};
MyAwesomeClass a{};
auto b = a; // no problem!
Features:
- Header only, single file, cross platform, no dependencies outside the STL
- Compatible interface/convertible to
std::unique_ptr
- Space efficient:
- Utilizes empty base optimization to minimize memory footprint
- defined types:
sizeof( value_ptr ) == sizeof(T*) == sizeof(std::unique_ptr)
- Undefined types:
sizeof( value_ptr ) == sizeof(T*)
+ two function pointers
- Polymorphic copying:
- Automatically detects/utilizes
clone()
member function
- Automatically detects/utilizes
- Static assertion prevents object slicing if a user-defined copier not provided or clone member not found
- Support for stateful and stateless deleters and copiers, via functors or lambdas
- Unit tested, valgrind clean
The code:
// Distributed under the Boost Software License, Version 1.0.
// (See http://www.boost.org/LICENSE_1_0.txt)
#ifndef _SMART_PTR_VALUE_PTR_
#define _SMART_PTR_VALUE_PTR_
#include <memory> // unique_ptr
#include <functional> // less_equal
#include <cassert> // assert
#if defined( _MSC_VER) // todo: check constexpr/delegating ctor issue in vs17. issue persists in vs15 update 3 despite ms closed bug as fixed, or i'm doing something wrong
#define VALUE_PTR_CONSTEXPR
#else
#define VALUE_PTR_CONSTEXPR constexpr
#endif
namespace smart_ptr {
namespace detail {
// void_t for c++11
// from http://en.cppreference.com/w/cpp/types/void_t
template<typename... Ts> struct make_void { typedef void type; };
template<typename... Ts> using void_t = typename make_void<Ts...>::type;
// is_defined<T>, from https://stackoverflow.com/a/39816909
template <class, class = void> struct is_defined : std::false_type { };
template <class T> struct is_defined<
T
, typename std::enable_if<std::is_object<T>::value && !std::is_pointer<T>::value && ( sizeof( T ) > 0 )>::type
>
: std::true_type{}
;
// Class function/type detection
// https://stackoverflow.com/a/30848101
// Primary template handles all types not supporting the operation.
template <typename, template <typename> class, typename = void_t<>>
struct detect : std::false_type {};
// Specialization recognizes/validates only types supporting the archetype.
template <typename T, template <typename> class Op>
struct detect<T, Op, void_t<Op<T>>> : std::true_type {};
// clone function
template <typename T> using fn_clone_t = decltype( std::declval<T>().clone() );
// has_clone
template <typename T> using has_clone = detect<T, fn_clone_t>;
// Returns flag if test passes (false==slicing is probable)
// T==base pointer, U==derived/supplied pointer
template <typename T, typename U, bool IsDefaultCopier>
struct slice_test : std::conditional<
std::is_same<T, U>::value // if U==T, no need to check for slicing
|| std::is_same<std::nullptr_t, U>::value // nullptr is fine
|| !IsDefaultCopier // user provided cloner, assume they're handling it
|| has_clone<typename std::remove_pointer<U>::type>::value // using default cloner, clone method must exist in U
, std::true_type
, std::false_type
>::type {};
// op_wrapper wraps Op::operator() and dispatches to observer fn
// observer fn then calls op_wrapper.op() to invoke Op::operator()
// this redirection is needed to call the actual operation in a context when T is actually defined
template <typename T, typename Op, typename R, typename ObserverFnSig>
struct op_wrapper : public Op {
using this_type = op_wrapper<T, Op, R, ObserverFnSig>;
using return_type = R;
// observer function to call
ObserverFnSig observer_fn;
template <typename Op_, typename Fn>
constexpr op_wrapper( Op_&& op, Fn&& obs )
: Op( std::forward<Op_>( op ) )
, observer_fn( std::forward<Fn>( obs ) )
{}
// invoked for event
template <typename... Args>
return_type operator()( Args&&... args ) const {
assert( this->observer_fn != nullptr );
// here we want to notify observer of event, with reference to this as first parameter
return this->observer_fn( (const void*)this, std::forward<Args>( args )... );
}
// call to actual operation (Op::operator()), invoked by observer
template <typename... Args>
return_type op( Args&&... args ) const {
return Op::operator()( std::forward<Args>(args)... );
}
}; // op_wrapper
// ptr_data
template <typename T, typename Deleter, typename Copier>
struct
#ifdef _MSC_VER
// https://blogs.msdn.microsoft.com/vcblog/2016/03/30/optimizing-the-layout-of-empty-base-classes-in-vs2015-update-2-3/
__declspec( empty_bases ) // requires vs2015 update 2
#endif
ptr_data
: public std::unique_ptr<T, Deleter>
, public Copier
{
using copier_type = Copier;
using base_type_uptr = std::unique_ptr<T, Deleter>;
using deleter_type = Deleter;
ptr_data() = default;
template <typename Dx, typename Cx>
constexpr ptr_data( T* px, Dx&& dx, Cx&& cx ) noexcept
: base_type_uptr( px, std::forward<Dx>(dx) )
, copier_type( std::forward<Cx>(cx) )
{}
copier_type& get_copier() { return static_cast<copier_type&>( *this ); }
const copier_type& get_copier() const { return static_cast<const copier_type&>( *this ); }
ptr_data clone() const {
return{ this->get_copier()( this->get() ), this->get_deleter(), this->get_copier() };
}
ptr_data( ptr_data&& ) = default;
ptr_data& operator=( ptr_data&& ) = default;
ptr_data( const ptr_data& that )
: ptr_data( that.clone() )
{}
ptr_data& operator=( const ptr_data& that ) {
if ( this == &that )
return *this;
*this = that.clone();
return *this;
}
}; // ptr_data
// ptr_base: base class for defined types
template <typename T, typename Deleter, typename Copier>
struct ptr_base {
using _data_type = ptr_data<T, Deleter, Copier>;
using _pointer = typename _data_type::pointer;
_data_type _data;
using pointer = _pointer;
template <typename Px, typename Dx, typename Cx>
constexpr ptr_base( Px&& px, Dx&& dx, Cx&& cx )
: _data(
std::forward<Px>( px )
, std::forward<Dx>( dx )
, std::forward<Cx>(cx)
)
{}
// conversion to unique_ptr
const typename _data_type::base_type_uptr& ptr() const {
return this->_data;
}
// conversion to unique_ptr
typename _data_type::base_type_uptr& ptr() {
return this->_data;
}
// conversion to unique_ptr
operator typename _data_type::base_type_uptr const&() const {
return this->_data;
}
// conversion to unique_ptr
operator typename _data_type::base_type_uptr& () {
return this->_data;
}
}; // ptr_base
// ptr_base_undefined: intermediate base class for undefined types
template <typename T, typename DeleteOp, typename CopyOp
, typename Deleter = op_wrapper<T, DeleteOp, void, void( *)( const void*, T* )>
, typename Copier = op_wrapper<T, CopyOp, T*, T*(*)(const void*, const T*)>
>
struct ptr_base_undefined
: ptr_base<T, Deleter, Copier> {
using base_type = ptr_base<T,Deleter,Copier>;
using pointer = typename base_type::pointer;
// default construct for undefined type
template <typename Dx, typename Cx>
constexpr ptr_base_undefined( std::nullptr_t, Dx&& dx, Cx&& cx )
: base_type(
nullptr
, Deleter( std::forward<Dx>( dx ), ( const void*, T* ptr ) { assert( ptr == nullptr ); } )
, Copier( std::forward<Cx>( cx ), ( const void* op, const T* ptr ) -> T* { assert( ptr == nullptr ); return nullptr; } )
)
{}
template <typename Dx, typename Cx>
constexpr ptr_base_undefined( pointer px, Dx&& dx, Cx&& cx )
: base_type(
px
, Deleter( std::forward<Dx>( dx ), ( const void* op, T* ptr ) {
if ( ptr )
static_cast<const Deleter*>( op )->op( ptr );
}
)
, Copier( std::forward<Cx>( cx ), ( const void* op, const T* ptr ) -> T* {
if ( !ptr )
return nullptr;
return static_cast<const Copier*>( op )->op( ptr );
}
)
)
{}
}; // ptr_base_undefined
} // detail
template <typename T>
struct default_copy {
// copy operator
T *operator()( const T* what ) const {
if ( !what )
return nullptr;
// tag dispatch on has_clone
return this->operator()( what
, typename std::conditional<detail::has_clone<T>::value, _clone, _copy>::type()
);
} // operator()
private:
struct _clone {};
struct _copy {};
T* operator()( const T* what, _clone ) const {
return what->clone();
}
T* operator()( const T* what, _copy ) const {
return new T( *what );
} // _copy
}; // default_copy
template <typename T
, typename Deleter = std::default_delete<T>
, typename Copier = default_copy<T>
, typename Base =
typename std::conditional<detail::is_defined<T>::value,
detail::ptr_base<T, Deleter, Copier>
, detail::ptr_base_undefined<T, Deleter, Copier>
>::type
>
struct value_ptr
: Base
{
using base_type = Base;
using element_type = T;
using pointer = typename base_type::pointer;
using const_pointer = const pointer;
using reference = typename std::add_lvalue_reference<element_type>::type;
using const_reference = const reference;
using deleter_type = Deleter;
using copier_type = Copier;
// construct with pointer, deleter, copier
template <typename Px>
constexpr value_ptr( Px px, deleter_type dx, copier_type cx )
: base_type( px
, std::move( dx )
, std::move( cx )
)
{
static_assert(
detail::slice_test<pointer, Px, std::is_same<default_copy<T>, copier_type>::value>::value
, "value_ptr; clone() method not detected and not using custom copier; slicing may occur"
);
}
// construct with pointer, deleter
template <typename Px>
VALUE_PTR_CONSTEXPR value_ptr( Px px, deleter_type dx ) // constexpr here yields c2476 on msvc15
: value_ptr( px, std::move(dx), copier_type() )
{}
// construct with pointer
template <typename Px>
VALUE_PTR_CONSTEXPR value_ptr( Px px ) // constexpr here yields c2476 on msvc15
: value_ptr( px, deleter_type(), copier_type() )
{}
// std::nullptr_t, default ctor
explicit VALUE_PTR_CONSTEXPR value_ptr( std::nullptr_t = nullptr ) // constexpr here yields c2476 on msvc15
: value_ptr( nullptr, deleter_type(), copier_type() )
{}
// get pointer
pointer get() { return this->_data.get(); }
// get const pointer
const_pointer get() const { return this->_data.get(); }
// reset pointer
template <typename Px = std::nullptr_t>
void reset( Px px = nullptr ) {
static_assert(
detail::slice_test<pointer, Px, std::is_same<default_copy<T>, copier_type>::value>::value
, "value_ptr; clone() method not detected and not using custom copier; slicing may occur"
);
*this = value_ptr( px, this->get_deleter(), this->get_copier() );
}
// release pointer
pointer release() noexcept {
return this->_data.release();
} // release
// return flag if has pointer
explicit operator bool() const {
return this->get() != nullptr;
}
const_reference operator*() const { return *this->get(); }
reference operator*() { return *this->get(); }
const_pointer operator-> () const { return this->get(); }
pointer operator-> () { return this->get(); }
void swap( value_ptr& that ) { std::swap( this->_data, that._data ); }
deleter_type& get_deleter() { return this->_data.get_deleter(); }
const deleter_type& get_deleter() const { return this->_data.get_deleter(); }
copier_type& get_copier() { return this->_data.get_copier(); }
const copier_type& get_copier() const { return this->_data.get_copier(); }
};// value_ptr
// non-member swap
template <class T1, class D1, class C1, class T2, class D2, class C2> void swap( value_ptr<T1, D1, C1>& x, value_ptr<T2, D2, C2>& y ) { x.swap( y ); }
// non-member operators
template <class T1, class D1, class C1, class T2, class D2, class C2> bool operator == ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return x.get() == y.get(); }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator != ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return x.get() != y.get(); }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator < ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) {
using common_type = typename std::common_type<typename value_ptr<T1, D1, C1>::pointer, typename value_ptr<T2, D2, C2>::pointer>::type;
return std::less<common_type>()( x.get(), y.get() );
}
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator <= ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return !( y < x ); }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator >( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return y < x; }
template<class T1, class D1, class C1, class T2, class D2, class C2> bool operator >= ( const value_ptr<T1, D1, C1>& x, const value_ptr<T2, D2, C2>& y ) { return !( x < y ); }
template <class T, class D, class C> bool operator == ( const value_ptr<T, D, C>& x, std::nullptr_t ) noexcept { return !x; }
template <class T, class D, class C> bool operator == ( std::nullptr_t, const value_ptr<T, D, C>& x ) noexcept { return !x; }
template <class T, class D, class C> bool operator != ( const value_ptr<T, D, C>& x, std::nullptr_t ) noexcept { return (bool)x; }
template <class T, class D, class C> bool operator != ( std::nullptr_t, const value_ptr<T, D, C>& x ) noexcept { return (bool)x; }
template <class T, class D, class C> bool operator < ( const value_ptr<T, D, C>& x, std::nullptr_t ) { return std::less<typename value_ptr<T, D, C>::pointer>()( x.get(), nullptr ); }
template <class T, class D, class C> bool operator<( std::nullptr_t, const value_ptr<T, D, C>& y ) { return std::less<typename value_ptr<T, D, C>::pointer>()( nullptr, y.get() ); }
template <class T, class D, class C> bool operator <= ( const value_ptr<T, D, C>& x, std::nullptr_t ) { return std::less_equal<typename value_ptr<T, D, C>::pointer>()( x.get(), nullptr ); }
template <class T, class D, class C> bool operator <= ( std::nullptr_t, const value_ptr<T, D, C>& y ) { return std::less_equal<typename value_ptr<T, D, C>::pointer>()( nullptr, y.get() ); }
template <class T, class D, class C> bool operator >( const value_ptr<T, D, C>& x, std::nullptr_t ) { return !( nullptr < x ); }
template <class T, class D, class C> bool operator > ( std::nullptr_t, const value_ptr<T, D, C>& y ) { return !( y < nullptr ); }
template <class T, class D, class C> bool operator >= ( const value_ptr<T, D, C>& x, std::nullptr_t ) { return !( x < nullptr ); }
template <class T, class D, class C> bool operator >= ( std::nullptr_t, const value_ptr<T, D, C>& y ) { return !( nullptr < y ); }
template <typename T, typename Deleter>
static inline auto make_value_ptr( T* ptr, Deleter&& dx ) -> value_ptr<T, Deleter> {
return value_ptr<T, Deleter>( ptr, std::forward<Deleter>( dx ) );
} // make_value_ptr
template <typename T, typename Deleter, typename Copier>
static inline auto make_value_ptr( T* ptr, Deleter&& dx, Copier&& cx ) -> value_ptr<T, Deleter, Copier> {
return value_ptr<T, Deleter, Copier>( ptr, std::forward<Deleter>( dx ), std::forward<Copier>( cx ) );
} // make_value_ptr
} // smart_ptr ns
#undef VALUE_PTR_CONSTEXPR
#endif // !_SMART_PTR_VALUE_PTR_
c++ c++11 library pointers
c++ c++11 library pointers
edited Jul 11 '17 at 22:48
asked Jul 11 '17 at 2:39
Tom
21614
21614
1
Shame nobody seems to have time to review this, it’s a very nice submission. Unfortunately I lack the time at the moment; just one thing I noticed; the identifier_SMART_PTR_VALUE_PTR_
is reserved for the implementation, and is illegal in client code; the rule is: everything in the global namespace starting with__
(double underscore) or_
(single underscore) + uppercase letter is reserved.
– Konrad Rudolph
Jul 14 '17 at 14:33
@KonradRudolph - thank you! I didn't know that about the leading underscore
– Tom
Jul 14 '17 at 16:33
Wow. I had never heard of__declspec( empty_bases )
before now. That's something I'm gonna have to use.
– Justin
Aug 2 '17 at 21:10
1
@Justin, there is standard solution coming, IIRC it is called[[no_unique_address]]
.
– Incomputable
Oct 30 at 5:33
add a comment |
1
Shame nobody seems to have time to review this, it’s a very nice submission. Unfortunately I lack the time at the moment; just one thing I noticed; the identifier_SMART_PTR_VALUE_PTR_
is reserved for the implementation, and is illegal in client code; the rule is: everything in the global namespace starting with__
(double underscore) or_
(single underscore) + uppercase letter is reserved.
– Konrad Rudolph
Jul 14 '17 at 14:33
@KonradRudolph - thank you! I didn't know that about the leading underscore
– Tom
Jul 14 '17 at 16:33
Wow. I had never heard of__declspec( empty_bases )
before now. That's something I'm gonna have to use.
– Justin
Aug 2 '17 at 21:10
1
@Justin, there is standard solution coming, IIRC it is called[[no_unique_address]]
.
– Incomputable
Oct 30 at 5:33
1
1
Shame nobody seems to have time to review this, it’s a very nice submission. Unfortunately I lack the time at the moment; just one thing I noticed; the identifier
_SMART_PTR_VALUE_PTR_
is reserved for the implementation, and is illegal in client code; the rule is: everything in the global namespace starting with __
(double underscore) or _
(single underscore) + uppercase letter is reserved.– Konrad Rudolph
Jul 14 '17 at 14:33
Shame nobody seems to have time to review this, it’s a very nice submission. Unfortunately I lack the time at the moment; just one thing I noticed; the identifier
_SMART_PTR_VALUE_PTR_
is reserved for the implementation, and is illegal in client code; the rule is: everything in the global namespace starting with __
(double underscore) or _
(single underscore) + uppercase letter is reserved.– Konrad Rudolph
Jul 14 '17 at 14:33
@KonradRudolph - thank you! I didn't know that about the leading underscore
– Tom
Jul 14 '17 at 16:33
@KonradRudolph - thank you! I didn't know that about the leading underscore
– Tom
Jul 14 '17 at 16:33
Wow. I had never heard of
__declspec( empty_bases )
before now. That's something I'm gonna have to use.– Justin
Aug 2 '17 at 21:10
Wow. I had never heard of
__declspec( empty_bases )
before now. That's something I'm gonna have to use.– Justin
Aug 2 '17 at 21:10
1
1
@Justin, there is standard solution coming, IIRC it is called
[[no_unique_address]]
.– Incomputable
Oct 30 at 5:33
@Justin, there is standard solution coming, IIRC it is called
[[no_unique_address]]
.– Incomputable
Oct 30 at 5:33
add a comment |
1 Answer
1
active
oldest
votes
up vote
4
down vote
Scanning your description: The idea of this value_ptr
sounds perfectly reasonable. You have a pointer to a polymorphic object, and then you want to be able to make a copy of that object, without necessarily knowing what its "most derived type" is. You can't do that without help; so we ask the derived type to help us out: we give the base type a virtual method clone()
through which we can ask the derived type to copy itself.
Your example shows clone()
returning an owning raw pointer:
struct Base { virtual Base* clone() const { return new Base(*this); } };
struct Derived : Base { Base* clone() const { return new Derived(*this); };
I strongly recommend making the signature std::unique_ptr<Base> clone() const
, so that the owning nature of the return value (and the fact that it is the sole owner) is expressed directly in the code instead of implicitly.
The one possible downside of using unique_ptr
is that you can no longer use covariant return types:
struct Base { virtual Base* clone() const; };
struct Derived : Base { Derived* clone() const override; }; // OK
struct Base { virtual std::unique_ptr<Base> clone() const; };
struct Derived : Base { std::unique_ptr<Derived> clone() const override; }; // error!
But this isn't a big downside in your case.
The other stylistic thing to notice about my example code above is that I'm using override
to tell the compiler that I intend to override a virtual method from one of my base classes, and so it should complain if my intention isn't being carried out for some reason.
struct Base { virtual Base* clone() const; };
struct Derived : Base { Derived* clone(); }; // Does The Wrong Thing at runtime
struct Base { virtual Base* clone() const; };
struct Derived : Base { Derived* clone() override; }; // error! hooray!
Finally — although I'm sure this was just an oversight in toy example code — don't forget that Base
's destructor should be virtual
, given that you're eventually going to be deleting a Derived
object via a pointer of type Base*
.
// is_defined<T>, from https://stackoverflow.com/a/39816909
template <class, class = void> struct is_defined : std::false_type { };
template <class T> struct is_defined<
T
, typename std::enable_if<std::is_object<T>::value && !std::is_pointer<T>::value && ( sizeof( T ) > 0 )>::type
>
: std::true_type{}
;
Ooh. You really don't want to be doing this. First of all, I think the "proper" name for this type-trait would be (the inverse of) is_incomplete<T>
; types don't really get "defined" per se. Or, okay, class types do; but for example void
and int
are types that are "defined" but still "incomplete," and they're the kinds of types you're catching with this type-trait.
Secondly, why don't you want to be doing this? Well, because the intended value of this trait can change over the course of a translation unit; but the actual value of the static data member cannot change.
struct A;
static_assert(is_incomplete<A>::value, "If this assertion succeeds...");
struct A {};
static_assert(not is_incomplete<A>::value, "...then this one MUST fail!");
(Godbolt.)
So don't do this. Trust your library-user to use your type correctly, and trust the compiler to give them a reasonable diagnostic if they misuse it. Don't try to dispatch on evanescent properties such as "completeness."
Finally, a nit on whitespace:
: std::true_type{}
;
The construct std::true_type{}
has a well-known meaning to C++11 metaprogrammers: it means "give me an object of type std::true_type
." What you mean in this case is not that — you mean "...inherits from true_type
, and here's the class body, which happens to be empty." So use your whitespace to indicate that.
: std::true_type {};
// or even
: std::true_type
{};
template <typename, template <typename> class, typename = void_t<>>
struct detect : std::false_type {};
That first line is a complicated way of writing
template<class, template<class> class, class = void>
Let's look at this whole snippet:
// Class function/type detection
// https://stackoverflow.com/a/30848101
// Primary template handles all types not supporting the operation.
template <typename, template <typename> class, typename = void_t<>>
struct detect : std::false_type {};
// Specialization recognizes/validates only types supporting the archetype.
template <typename T, template <typename> class Op>
struct detect<T, Op, void_t<Op<T>>> : std::true_type {};
// clone function
template <typename T> using fn_clone_t = decltype( std::declval<T>().clone() );
// has_clone
template <typename T> using has_clone = detect<T, fn_clone_t>;
This is an insanely complicated way of writing what should be a two-liner:
template<class T, class = void> struct has_clone : std::false_type {};
template<class T> struct has_clone<T, decltype(void(std::declval<T>().clone()))> : std::true_type {};
Consider this line of metaprogramming. (I'm not going to worry about what it does, for now.)
template <typename T, typename U, bool IsDefaultCopier>
struct slice_test : std::conditional<
std::is_same<T, U>::value // if U==T, no need to check for slicing
|| std::is_same<std::nullptr_t, U>::value // nullptr is fine
|| !IsDefaultCopier // user provided cloner, assume they're handling it
|| has_clone<typename std::remove_pointer<U>::type>::value // using default cloner, clone method must exist in U
, std::true_type
, std::false_type
>::type {};
Would you ever write
return ((t == u) || (nullptr == u) || !isdefaultcopier || has_clone(u))
? true : false;
? No? Then you shouldn't write the metaprogramming equivalent of return x ? true : false;
either. Just return the original boolean condition itself:
template <typename T, typename U, bool IsDefaultCopier>
struct slice_test : std::bool_constant<
std::is_same_v<T, U> // if U==T, no need to check for slicing
|| std::is_same_v<std::nullptr_t, U> // nullptr is fine
|| !IsDefaultCopier // user provided cloner, assume they're handling it
|| has_clone<std::remove_pointer_t<U>>::value // using default cloner, clone method must exist in U
> {};
(For C++11, use std::integral_constant<bool, XYZ>
in place of std::bool_constant<XYZ>
, and re-expand all my _t
s and _v
s. I shrank them here just to demonstrate how code should look if portability-to-C++11 is not a concern.)
struct
#ifdef _MSC_VER
// https://blogs.msdn.microsoft.com/vcblog/2016/03/30/optimizing-the-layout-of-empty-base-classes-in-vs2015-update-2-3/
__declspec( empty_bases ) // requires vs2015 update 2
#endif
ptr_data
I strongly recommend that you move the preprocessor logic to the top of the file, use it to set a macro, and use just the macro down here. It'll be easier to read and easier to maintain.
#ifdef _MSC_VER
#define USE_EMPTY_BASE_OPTIMIZATION __declspec(empty_bases)
#else
#define USE_EMPTY_BASE_OPTIMIZATION
#endif
// ...
struct USE_EMPTY_BASE_OPTIMIZATION ptr_data
: public std::unique_ptr<T, Deleter>
Inheriting (either publicly or privately) from standard-library types is never a good idea. I strongly recommend that you just implement the three special members (move-constructor, move-assignment, and destructor) yourself, rather than trying to reuse the STL's versions. ...Oh look, you do reimplement them, anyway!
ptr_data& operator=( const ptr_data& that ) {
if ( this == &that )
return *this;
*this = that.clone(); // R
return *this;
}
It's been a while since you wrote this code. How long will it take you to prove to yourself that the line marked // R
is not a recursive call to this operator=
? (Or is it? ;) Remember all of the behaviors of unique_ptr
!)
Speaking of which, inheriting from unique_ptr<T, Deleter>
is also kind of silly since unique_ptr<T, Deleter>
will have a data member of type Deleter::pointer
(which may be a fancy pointer type), whereas your clone()
methods seem to deal only in raw T*
. If you don't want the fancy-pointer behavior, you shouldn't drag it in. Yet another reason to implement the special members yourself instead of inheriting from unique_ptr
.
// tag dispatch on has_clone
return this->operator()( what
, typename std::conditional<detail::has_clone<T>::value, _clone, _copy>::type()
);
You seem to be a bit operator()
-happy in this code. There's no reason for this overload set to be named operator()
; I'd recommend something descriptive such as copy_or_clone
.
General rule of thumb: If you find yourself writing this->operator()(...)
instead of (*this)(...)
, then you are definitely doing it wrong. (And if you find yourself writing (*this)(...)
, you are probably doing it wrong, anyway.)
I also strongly recommend to avoid leading underscores. In this case, since they are tag types, _copy
should be spelled copy_tag
and _clone
should be spelled clone_tag
.
Did you consider using std::function<Base*(Base*)> m_clone
and std::function<void(Base*)> m_destroy
instead of rolling your own type-erasure? I mean, I love rolling my own type-erasure, and do it all the time; but if you're just practicing metaprogramming and want the shortest possible code, you might find that std::function
would be useful.
You could even use std::function<Base*(Base*, enum CloneOrDestroy)>
to wrap up both behaviors into a single function, to shrink the size of your value_ptr
object.
Did you consider what should happen if Base
's destructor is not virtual
? There is actually a type-trait for has_virtual_destructor<T>
, not that I would recommend using it (see my above advice about trusting your user). But consider how shared_ptr
works even with non-virtual destructors; should you do similarly?
This has been a long review already, and I didn't even really get to the meat of it. I would recommend simplifying the code a lot and re-posting for another round.
add a comment |
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
4
down vote
Scanning your description: The idea of this value_ptr
sounds perfectly reasonable. You have a pointer to a polymorphic object, and then you want to be able to make a copy of that object, without necessarily knowing what its "most derived type" is. You can't do that without help; so we ask the derived type to help us out: we give the base type a virtual method clone()
through which we can ask the derived type to copy itself.
Your example shows clone()
returning an owning raw pointer:
struct Base { virtual Base* clone() const { return new Base(*this); } };
struct Derived : Base { Base* clone() const { return new Derived(*this); };
I strongly recommend making the signature std::unique_ptr<Base> clone() const
, so that the owning nature of the return value (and the fact that it is the sole owner) is expressed directly in the code instead of implicitly.
The one possible downside of using unique_ptr
is that you can no longer use covariant return types:
struct Base { virtual Base* clone() const; };
struct Derived : Base { Derived* clone() const override; }; // OK
struct Base { virtual std::unique_ptr<Base> clone() const; };
struct Derived : Base { std::unique_ptr<Derived> clone() const override; }; // error!
But this isn't a big downside in your case.
The other stylistic thing to notice about my example code above is that I'm using override
to tell the compiler that I intend to override a virtual method from one of my base classes, and so it should complain if my intention isn't being carried out for some reason.
struct Base { virtual Base* clone() const; };
struct Derived : Base { Derived* clone(); }; // Does The Wrong Thing at runtime
struct Base { virtual Base* clone() const; };
struct Derived : Base { Derived* clone() override; }; // error! hooray!
Finally — although I'm sure this was just an oversight in toy example code — don't forget that Base
's destructor should be virtual
, given that you're eventually going to be deleting a Derived
object via a pointer of type Base*
.
// is_defined<T>, from https://stackoverflow.com/a/39816909
template <class, class = void> struct is_defined : std::false_type { };
template <class T> struct is_defined<
T
, typename std::enable_if<std::is_object<T>::value && !std::is_pointer<T>::value && ( sizeof( T ) > 0 )>::type
>
: std::true_type{}
;
Ooh. You really don't want to be doing this. First of all, I think the "proper" name for this type-trait would be (the inverse of) is_incomplete<T>
; types don't really get "defined" per se. Or, okay, class types do; but for example void
and int
are types that are "defined" but still "incomplete," and they're the kinds of types you're catching with this type-trait.
Secondly, why don't you want to be doing this? Well, because the intended value of this trait can change over the course of a translation unit; but the actual value of the static data member cannot change.
struct A;
static_assert(is_incomplete<A>::value, "If this assertion succeeds...");
struct A {};
static_assert(not is_incomplete<A>::value, "...then this one MUST fail!");
(Godbolt.)
So don't do this. Trust your library-user to use your type correctly, and trust the compiler to give them a reasonable diagnostic if they misuse it. Don't try to dispatch on evanescent properties such as "completeness."
Finally, a nit on whitespace:
: std::true_type{}
;
The construct std::true_type{}
has a well-known meaning to C++11 metaprogrammers: it means "give me an object of type std::true_type
." What you mean in this case is not that — you mean "...inherits from true_type
, and here's the class body, which happens to be empty." So use your whitespace to indicate that.
: std::true_type {};
// or even
: std::true_type
{};
template <typename, template <typename> class, typename = void_t<>>
struct detect : std::false_type {};
That first line is a complicated way of writing
template<class, template<class> class, class = void>
Let's look at this whole snippet:
// Class function/type detection
// https://stackoverflow.com/a/30848101
// Primary template handles all types not supporting the operation.
template <typename, template <typename> class, typename = void_t<>>
struct detect : std::false_type {};
// Specialization recognizes/validates only types supporting the archetype.
template <typename T, template <typename> class Op>
struct detect<T, Op, void_t<Op<T>>> : std::true_type {};
// clone function
template <typename T> using fn_clone_t = decltype( std::declval<T>().clone() );
// has_clone
template <typename T> using has_clone = detect<T, fn_clone_t>;
This is an insanely complicated way of writing what should be a two-liner:
template<class T, class = void> struct has_clone : std::false_type {};
template<class T> struct has_clone<T, decltype(void(std::declval<T>().clone()))> : std::true_type {};
Consider this line of metaprogramming. (I'm not going to worry about what it does, for now.)
template <typename T, typename U, bool IsDefaultCopier>
struct slice_test : std::conditional<
std::is_same<T, U>::value // if U==T, no need to check for slicing
|| std::is_same<std::nullptr_t, U>::value // nullptr is fine
|| !IsDefaultCopier // user provided cloner, assume they're handling it
|| has_clone<typename std::remove_pointer<U>::type>::value // using default cloner, clone method must exist in U
, std::true_type
, std::false_type
>::type {};
Would you ever write
return ((t == u) || (nullptr == u) || !isdefaultcopier || has_clone(u))
? true : false;
? No? Then you shouldn't write the metaprogramming equivalent of return x ? true : false;
either. Just return the original boolean condition itself:
template <typename T, typename U, bool IsDefaultCopier>
struct slice_test : std::bool_constant<
std::is_same_v<T, U> // if U==T, no need to check for slicing
|| std::is_same_v<std::nullptr_t, U> // nullptr is fine
|| !IsDefaultCopier // user provided cloner, assume they're handling it
|| has_clone<std::remove_pointer_t<U>>::value // using default cloner, clone method must exist in U
> {};
(For C++11, use std::integral_constant<bool, XYZ>
in place of std::bool_constant<XYZ>
, and re-expand all my _t
s and _v
s. I shrank them here just to demonstrate how code should look if portability-to-C++11 is not a concern.)
struct
#ifdef _MSC_VER
// https://blogs.msdn.microsoft.com/vcblog/2016/03/30/optimizing-the-layout-of-empty-base-classes-in-vs2015-update-2-3/
__declspec( empty_bases ) // requires vs2015 update 2
#endif
ptr_data
I strongly recommend that you move the preprocessor logic to the top of the file, use it to set a macro, and use just the macro down here. It'll be easier to read and easier to maintain.
#ifdef _MSC_VER
#define USE_EMPTY_BASE_OPTIMIZATION __declspec(empty_bases)
#else
#define USE_EMPTY_BASE_OPTIMIZATION
#endif
// ...
struct USE_EMPTY_BASE_OPTIMIZATION ptr_data
: public std::unique_ptr<T, Deleter>
Inheriting (either publicly or privately) from standard-library types is never a good idea. I strongly recommend that you just implement the three special members (move-constructor, move-assignment, and destructor) yourself, rather than trying to reuse the STL's versions. ...Oh look, you do reimplement them, anyway!
ptr_data& operator=( const ptr_data& that ) {
if ( this == &that )
return *this;
*this = that.clone(); // R
return *this;
}
It's been a while since you wrote this code. How long will it take you to prove to yourself that the line marked // R
is not a recursive call to this operator=
? (Or is it? ;) Remember all of the behaviors of unique_ptr
!)
Speaking of which, inheriting from unique_ptr<T, Deleter>
is also kind of silly since unique_ptr<T, Deleter>
will have a data member of type Deleter::pointer
(which may be a fancy pointer type), whereas your clone()
methods seem to deal only in raw T*
. If you don't want the fancy-pointer behavior, you shouldn't drag it in. Yet another reason to implement the special members yourself instead of inheriting from unique_ptr
.
// tag dispatch on has_clone
return this->operator()( what
, typename std::conditional<detail::has_clone<T>::value, _clone, _copy>::type()
);
You seem to be a bit operator()
-happy in this code. There's no reason for this overload set to be named operator()
; I'd recommend something descriptive such as copy_or_clone
.
General rule of thumb: If you find yourself writing this->operator()(...)
instead of (*this)(...)
, then you are definitely doing it wrong. (And if you find yourself writing (*this)(...)
, you are probably doing it wrong, anyway.)
I also strongly recommend to avoid leading underscores. In this case, since they are tag types, _copy
should be spelled copy_tag
and _clone
should be spelled clone_tag
.
Did you consider using std::function<Base*(Base*)> m_clone
and std::function<void(Base*)> m_destroy
instead of rolling your own type-erasure? I mean, I love rolling my own type-erasure, and do it all the time; but if you're just practicing metaprogramming and want the shortest possible code, you might find that std::function
would be useful.
You could even use std::function<Base*(Base*, enum CloneOrDestroy)>
to wrap up both behaviors into a single function, to shrink the size of your value_ptr
object.
Did you consider what should happen if Base
's destructor is not virtual
? There is actually a type-trait for has_virtual_destructor<T>
, not that I would recommend using it (see my above advice about trusting your user). But consider how shared_ptr
works even with non-virtual destructors; should you do similarly?
This has been a long review already, and I didn't even really get to the meat of it. I would recommend simplifying the code a lot and re-posting for another round.
add a comment |
up vote
4
down vote
Scanning your description: The idea of this value_ptr
sounds perfectly reasonable. You have a pointer to a polymorphic object, and then you want to be able to make a copy of that object, without necessarily knowing what its "most derived type" is. You can't do that without help; so we ask the derived type to help us out: we give the base type a virtual method clone()
through which we can ask the derived type to copy itself.
Your example shows clone()
returning an owning raw pointer:
struct Base { virtual Base* clone() const { return new Base(*this); } };
struct Derived : Base { Base* clone() const { return new Derived(*this); };
I strongly recommend making the signature std::unique_ptr<Base> clone() const
, so that the owning nature of the return value (and the fact that it is the sole owner) is expressed directly in the code instead of implicitly.
The one possible downside of using unique_ptr
is that you can no longer use covariant return types:
struct Base { virtual Base* clone() const; };
struct Derived : Base { Derived* clone() const override; }; // OK
struct Base { virtual std::unique_ptr<Base> clone() const; };
struct Derived : Base { std::unique_ptr<Derived> clone() const override; }; // error!
But this isn't a big downside in your case.
The other stylistic thing to notice about my example code above is that I'm using override
to tell the compiler that I intend to override a virtual method from one of my base classes, and so it should complain if my intention isn't being carried out for some reason.
struct Base { virtual Base* clone() const; };
struct Derived : Base { Derived* clone(); }; // Does The Wrong Thing at runtime
struct Base { virtual Base* clone() const; };
struct Derived : Base { Derived* clone() override; }; // error! hooray!
Finally — although I'm sure this was just an oversight in toy example code — don't forget that Base
's destructor should be virtual
, given that you're eventually going to be deleting a Derived
object via a pointer of type Base*
.
// is_defined<T>, from https://stackoverflow.com/a/39816909
template <class, class = void> struct is_defined : std::false_type { };
template <class T> struct is_defined<
T
, typename std::enable_if<std::is_object<T>::value && !std::is_pointer<T>::value && ( sizeof( T ) > 0 )>::type
>
: std::true_type{}
;
Ooh. You really don't want to be doing this. First of all, I think the "proper" name for this type-trait would be (the inverse of) is_incomplete<T>
; types don't really get "defined" per se. Or, okay, class types do; but for example void
and int
are types that are "defined" but still "incomplete," and they're the kinds of types you're catching with this type-trait.
Secondly, why don't you want to be doing this? Well, because the intended value of this trait can change over the course of a translation unit; but the actual value of the static data member cannot change.
struct A;
static_assert(is_incomplete<A>::value, "If this assertion succeeds...");
struct A {};
static_assert(not is_incomplete<A>::value, "...then this one MUST fail!");
(Godbolt.)
So don't do this. Trust your library-user to use your type correctly, and trust the compiler to give them a reasonable diagnostic if they misuse it. Don't try to dispatch on evanescent properties such as "completeness."
Finally, a nit on whitespace:
: std::true_type{}
;
The construct std::true_type{}
has a well-known meaning to C++11 metaprogrammers: it means "give me an object of type std::true_type
." What you mean in this case is not that — you mean "...inherits from true_type
, and here's the class body, which happens to be empty." So use your whitespace to indicate that.
: std::true_type {};
// or even
: std::true_type
{};
template <typename, template <typename> class, typename = void_t<>>
struct detect : std::false_type {};
That first line is a complicated way of writing
template<class, template<class> class, class = void>
Let's look at this whole snippet:
// Class function/type detection
// https://stackoverflow.com/a/30848101
// Primary template handles all types not supporting the operation.
template <typename, template <typename> class, typename = void_t<>>
struct detect : std::false_type {};
// Specialization recognizes/validates only types supporting the archetype.
template <typename T, template <typename> class Op>
struct detect<T, Op, void_t<Op<T>>> : std::true_type {};
// clone function
template <typename T> using fn_clone_t = decltype( std::declval<T>().clone() );
// has_clone
template <typename T> using has_clone = detect<T, fn_clone_t>;
This is an insanely complicated way of writing what should be a two-liner:
template<class T, class = void> struct has_clone : std::false_type {};
template<class T> struct has_clone<T, decltype(void(std::declval<T>().clone()))> : std::true_type {};
Consider this line of metaprogramming. (I'm not going to worry about what it does, for now.)
template <typename T, typename U, bool IsDefaultCopier>
struct slice_test : std::conditional<
std::is_same<T, U>::value // if U==T, no need to check for slicing
|| std::is_same<std::nullptr_t, U>::value // nullptr is fine
|| !IsDefaultCopier // user provided cloner, assume they're handling it
|| has_clone<typename std::remove_pointer<U>::type>::value // using default cloner, clone method must exist in U
, std::true_type
, std::false_type
>::type {};
Would you ever write
return ((t == u) || (nullptr == u) || !isdefaultcopier || has_clone(u))
? true : false;
? No? Then you shouldn't write the metaprogramming equivalent of return x ? true : false;
either. Just return the original boolean condition itself:
template <typename T, typename U, bool IsDefaultCopier>
struct slice_test : std::bool_constant<
std::is_same_v<T, U> // if U==T, no need to check for slicing
|| std::is_same_v<std::nullptr_t, U> // nullptr is fine
|| !IsDefaultCopier // user provided cloner, assume they're handling it
|| has_clone<std::remove_pointer_t<U>>::value // using default cloner, clone method must exist in U
> {};
(For C++11, use std::integral_constant<bool, XYZ>
in place of std::bool_constant<XYZ>
, and re-expand all my _t
s and _v
s. I shrank them here just to demonstrate how code should look if portability-to-C++11 is not a concern.)
struct
#ifdef _MSC_VER
// https://blogs.msdn.microsoft.com/vcblog/2016/03/30/optimizing-the-layout-of-empty-base-classes-in-vs2015-update-2-3/
__declspec( empty_bases ) // requires vs2015 update 2
#endif
ptr_data
I strongly recommend that you move the preprocessor logic to the top of the file, use it to set a macro, and use just the macro down here. It'll be easier to read and easier to maintain.
#ifdef _MSC_VER
#define USE_EMPTY_BASE_OPTIMIZATION __declspec(empty_bases)
#else
#define USE_EMPTY_BASE_OPTIMIZATION
#endif
// ...
struct USE_EMPTY_BASE_OPTIMIZATION ptr_data
: public std::unique_ptr<T, Deleter>
Inheriting (either publicly or privately) from standard-library types is never a good idea. I strongly recommend that you just implement the three special members (move-constructor, move-assignment, and destructor) yourself, rather than trying to reuse the STL's versions. ...Oh look, you do reimplement them, anyway!
ptr_data& operator=( const ptr_data& that ) {
if ( this == &that )
return *this;
*this = that.clone(); // R
return *this;
}
It's been a while since you wrote this code. How long will it take you to prove to yourself that the line marked // R
is not a recursive call to this operator=
? (Or is it? ;) Remember all of the behaviors of unique_ptr
!)
Speaking of which, inheriting from unique_ptr<T, Deleter>
is also kind of silly since unique_ptr<T, Deleter>
will have a data member of type Deleter::pointer
(which may be a fancy pointer type), whereas your clone()
methods seem to deal only in raw T*
. If you don't want the fancy-pointer behavior, you shouldn't drag it in. Yet another reason to implement the special members yourself instead of inheriting from unique_ptr
.
// tag dispatch on has_clone
return this->operator()( what
, typename std::conditional<detail::has_clone<T>::value, _clone, _copy>::type()
);
You seem to be a bit operator()
-happy in this code. There's no reason for this overload set to be named operator()
; I'd recommend something descriptive such as copy_or_clone
.
General rule of thumb: If you find yourself writing this->operator()(...)
instead of (*this)(...)
, then you are definitely doing it wrong. (And if you find yourself writing (*this)(...)
, you are probably doing it wrong, anyway.)
I also strongly recommend to avoid leading underscores. In this case, since they are tag types, _copy
should be spelled copy_tag
and _clone
should be spelled clone_tag
.
Did you consider using std::function<Base*(Base*)> m_clone
and std::function<void(Base*)> m_destroy
instead of rolling your own type-erasure? I mean, I love rolling my own type-erasure, and do it all the time; but if you're just practicing metaprogramming and want the shortest possible code, you might find that std::function
would be useful.
You could even use std::function<Base*(Base*, enum CloneOrDestroy)>
to wrap up both behaviors into a single function, to shrink the size of your value_ptr
object.
Did you consider what should happen if Base
's destructor is not virtual
? There is actually a type-trait for has_virtual_destructor<T>
, not that I would recommend using it (see my above advice about trusting your user). But consider how shared_ptr
works even with non-virtual destructors; should you do similarly?
This has been a long review already, and I didn't even really get to the meat of it. I would recommend simplifying the code a lot and re-posting for another round.
add a comment |
up vote
4
down vote
up vote
4
down vote
Scanning your description: The idea of this value_ptr
sounds perfectly reasonable. You have a pointer to a polymorphic object, and then you want to be able to make a copy of that object, without necessarily knowing what its "most derived type" is. You can't do that without help; so we ask the derived type to help us out: we give the base type a virtual method clone()
through which we can ask the derived type to copy itself.
Your example shows clone()
returning an owning raw pointer:
struct Base { virtual Base* clone() const { return new Base(*this); } };
struct Derived : Base { Base* clone() const { return new Derived(*this); };
I strongly recommend making the signature std::unique_ptr<Base> clone() const
, so that the owning nature of the return value (and the fact that it is the sole owner) is expressed directly in the code instead of implicitly.
The one possible downside of using unique_ptr
is that you can no longer use covariant return types:
struct Base { virtual Base* clone() const; };
struct Derived : Base { Derived* clone() const override; }; // OK
struct Base { virtual std::unique_ptr<Base> clone() const; };
struct Derived : Base { std::unique_ptr<Derived> clone() const override; }; // error!
But this isn't a big downside in your case.
The other stylistic thing to notice about my example code above is that I'm using override
to tell the compiler that I intend to override a virtual method from one of my base classes, and so it should complain if my intention isn't being carried out for some reason.
struct Base { virtual Base* clone() const; };
struct Derived : Base { Derived* clone(); }; // Does The Wrong Thing at runtime
struct Base { virtual Base* clone() const; };
struct Derived : Base { Derived* clone() override; }; // error! hooray!
Finally — although I'm sure this was just an oversight in toy example code — don't forget that Base
's destructor should be virtual
, given that you're eventually going to be deleting a Derived
object via a pointer of type Base*
.
// is_defined<T>, from https://stackoverflow.com/a/39816909
template <class, class = void> struct is_defined : std::false_type { };
template <class T> struct is_defined<
T
, typename std::enable_if<std::is_object<T>::value && !std::is_pointer<T>::value && ( sizeof( T ) > 0 )>::type
>
: std::true_type{}
;
Ooh. You really don't want to be doing this. First of all, I think the "proper" name for this type-trait would be (the inverse of) is_incomplete<T>
; types don't really get "defined" per se. Or, okay, class types do; but for example void
and int
are types that are "defined" but still "incomplete," and they're the kinds of types you're catching with this type-trait.
Secondly, why don't you want to be doing this? Well, because the intended value of this trait can change over the course of a translation unit; but the actual value of the static data member cannot change.
struct A;
static_assert(is_incomplete<A>::value, "If this assertion succeeds...");
struct A {};
static_assert(not is_incomplete<A>::value, "...then this one MUST fail!");
(Godbolt.)
So don't do this. Trust your library-user to use your type correctly, and trust the compiler to give them a reasonable diagnostic if they misuse it. Don't try to dispatch on evanescent properties such as "completeness."
Finally, a nit on whitespace:
: std::true_type{}
;
The construct std::true_type{}
has a well-known meaning to C++11 metaprogrammers: it means "give me an object of type std::true_type
." What you mean in this case is not that — you mean "...inherits from true_type
, and here's the class body, which happens to be empty." So use your whitespace to indicate that.
: std::true_type {};
// or even
: std::true_type
{};
template <typename, template <typename> class, typename = void_t<>>
struct detect : std::false_type {};
That first line is a complicated way of writing
template<class, template<class> class, class = void>
Let's look at this whole snippet:
// Class function/type detection
// https://stackoverflow.com/a/30848101
// Primary template handles all types not supporting the operation.
template <typename, template <typename> class, typename = void_t<>>
struct detect : std::false_type {};
// Specialization recognizes/validates only types supporting the archetype.
template <typename T, template <typename> class Op>
struct detect<T, Op, void_t<Op<T>>> : std::true_type {};
// clone function
template <typename T> using fn_clone_t = decltype( std::declval<T>().clone() );
// has_clone
template <typename T> using has_clone = detect<T, fn_clone_t>;
This is an insanely complicated way of writing what should be a two-liner:
template<class T, class = void> struct has_clone : std::false_type {};
template<class T> struct has_clone<T, decltype(void(std::declval<T>().clone()))> : std::true_type {};
Consider this line of metaprogramming. (I'm not going to worry about what it does, for now.)
template <typename T, typename U, bool IsDefaultCopier>
struct slice_test : std::conditional<
std::is_same<T, U>::value // if U==T, no need to check for slicing
|| std::is_same<std::nullptr_t, U>::value // nullptr is fine
|| !IsDefaultCopier // user provided cloner, assume they're handling it
|| has_clone<typename std::remove_pointer<U>::type>::value // using default cloner, clone method must exist in U
, std::true_type
, std::false_type
>::type {};
Would you ever write
return ((t == u) || (nullptr == u) || !isdefaultcopier || has_clone(u))
? true : false;
? No? Then you shouldn't write the metaprogramming equivalent of return x ? true : false;
either. Just return the original boolean condition itself:
template <typename T, typename U, bool IsDefaultCopier>
struct slice_test : std::bool_constant<
std::is_same_v<T, U> // if U==T, no need to check for slicing
|| std::is_same_v<std::nullptr_t, U> // nullptr is fine
|| !IsDefaultCopier // user provided cloner, assume they're handling it
|| has_clone<std::remove_pointer_t<U>>::value // using default cloner, clone method must exist in U
> {};
(For C++11, use std::integral_constant<bool, XYZ>
in place of std::bool_constant<XYZ>
, and re-expand all my _t
s and _v
s. I shrank them here just to demonstrate how code should look if portability-to-C++11 is not a concern.)
struct
#ifdef _MSC_VER
// https://blogs.msdn.microsoft.com/vcblog/2016/03/30/optimizing-the-layout-of-empty-base-classes-in-vs2015-update-2-3/
__declspec( empty_bases ) // requires vs2015 update 2
#endif
ptr_data
I strongly recommend that you move the preprocessor logic to the top of the file, use it to set a macro, and use just the macro down here. It'll be easier to read and easier to maintain.
#ifdef _MSC_VER
#define USE_EMPTY_BASE_OPTIMIZATION __declspec(empty_bases)
#else
#define USE_EMPTY_BASE_OPTIMIZATION
#endif
// ...
struct USE_EMPTY_BASE_OPTIMIZATION ptr_data
: public std::unique_ptr<T, Deleter>
Inheriting (either publicly or privately) from standard-library types is never a good idea. I strongly recommend that you just implement the three special members (move-constructor, move-assignment, and destructor) yourself, rather than trying to reuse the STL's versions. ...Oh look, you do reimplement them, anyway!
ptr_data& operator=( const ptr_data& that ) {
if ( this == &that )
return *this;
*this = that.clone(); // R
return *this;
}
It's been a while since you wrote this code. How long will it take you to prove to yourself that the line marked // R
is not a recursive call to this operator=
? (Or is it? ;) Remember all of the behaviors of unique_ptr
!)
Speaking of which, inheriting from unique_ptr<T, Deleter>
is also kind of silly since unique_ptr<T, Deleter>
will have a data member of type Deleter::pointer
(which may be a fancy pointer type), whereas your clone()
methods seem to deal only in raw T*
. If you don't want the fancy-pointer behavior, you shouldn't drag it in. Yet another reason to implement the special members yourself instead of inheriting from unique_ptr
.
// tag dispatch on has_clone
return this->operator()( what
, typename std::conditional<detail::has_clone<T>::value, _clone, _copy>::type()
);
You seem to be a bit operator()
-happy in this code. There's no reason for this overload set to be named operator()
; I'd recommend something descriptive such as copy_or_clone
.
General rule of thumb: If you find yourself writing this->operator()(...)
instead of (*this)(...)
, then you are definitely doing it wrong. (And if you find yourself writing (*this)(...)
, you are probably doing it wrong, anyway.)
I also strongly recommend to avoid leading underscores. In this case, since they are tag types, _copy
should be spelled copy_tag
and _clone
should be spelled clone_tag
.
Did you consider using std::function<Base*(Base*)> m_clone
and std::function<void(Base*)> m_destroy
instead of rolling your own type-erasure? I mean, I love rolling my own type-erasure, and do it all the time; but if you're just practicing metaprogramming and want the shortest possible code, you might find that std::function
would be useful.
You could even use std::function<Base*(Base*, enum CloneOrDestroy)>
to wrap up both behaviors into a single function, to shrink the size of your value_ptr
object.
Did you consider what should happen if Base
's destructor is not virtual
? There is actually a type-trait for has_virtual_destructor<T>
, not that I would recommend using it (see my above advice about trusting your user). But consider how shared_ptr
works even with non-virtual destructors; should you do similarly?
This has been a long review already, and I didn't even really get to the meat of it. I would recommend simplifying the code a lot and re-posting for another round.
Scanning your description: The idea of this value_ptr
sounds perfectly reasonable. You have a pointer to a polymorphic object, and then you want to be able to make a copy of that object, without necessarily knowing what its "most derived type" is. You can't do that without help; so we ask the derived type to help us out: we give the base type a virtual method clone()
through which we can ask the derived type to copy itself.
Your example shows clone()
returning an owning raw pointer:
struct Base { virtual Base* clone() const { return new Base(*this); } };
struct Derived : Base { Base* clone() const { return new Derived(*this); };
I strongly recommend making the signature std::unique_ptr<Base> clone() const
, so that the owning nature of the return value (and the fact that it is the sole owner) is expressed directly in the code instead of implicitly.
The one possible downside of using unique_ptr
is that you can no longer use covariant return types:
struct Base { virtual Base* clone() const; };
struct Derived : Base { Derived* clone() const override; }; // OK
struct Base { virtual std::unique_ptr<Base> clone() const; };
struct Derived : Base { std::unique_ptr<Derived> clone() const override; }; // error!
But this isn't a big downside in your case.
The other stylistic thing to notice about my example code above is that I'm using override
to tell the compiler that I intend to override a virtual method from one of my base classes, and so it should complain if my intention isn't being carried out for some reason.
struct Base { virtual Base* clone() const; };
struct Derived : Base { Derived* clone(); }; // Does The Wrong Thing at runtime
struct Base { virtual Base* clone() const; };
struct Derived : Base { Derived* clone() override; }; // error! hooray!
Finally — although I'm sure this was just an oversight in toy example code — don't forget that Base
's destructor should be virtual
, given that you're eventually going to be deleting a Derived
object via a pointer of type Base*
.
// is_defined<T>, from https://stackoverflow.com/a/39816909
template <class, class = void> struct is_defined : std::false_type { };
template <class T> struct is_defined<
T
, typename std::enable_if<std::is_object<T>::value && !std::is_pointer<T>::value && ( sizeof( T ) > 0 )>::type
>
: std::true_type{}
;
Ooh. You really don't want to be doing this. First of all, I think the "proper" name for this type-trait would be (the inverse of) is_incomplete<T>
; types don't really get "defined" per se. Or, okay, class types do; but for example void
and int
are types that are "defined" but still "incomplete," and they're the kinds of types you're catching with this type-trait.
Secondly, why don't you want to be doing this? Well, because the intended value of this trait can change over the course of a translation unit; but the actual value of the static data member cannot change.
struct A;
static_assert(is_incomplete<A>::value, "If this assertion succeeds...");
struct A {};
static_assert(not is_incomplete<A>::value, "...then this one MUST fail!");
(Godbolt.)
So don't do this. Trust your library-user to use your type correctly, and trust the compiler to give them a reasonable diagnostic if they misuse it. Don't try to dispatch on evanescent properties such as "completeness."
Finally, a nit on whitespace:
: std::true_type{}
;
The construct std::true_type{}
has a well-known meaning to C++11 metaprogrammers: it means "give me an object of type std::true_type
." What you mean in this case is not that — you mean "...inherits from true_type
, and here's the class body, which happens to be empty." So use your whitespace to indicate that.
: std::true_type {};
// or even
: std::true_type
{};
template <typename, template <typename> class, typename = void_t<>>
struct detect : std::false_type {};
That first line is a complicated way of writing
template<class, template<class> class, class = void>
Let's look at this whole snippet:
// Class function/type detection
// https://stackoverflow.com/a/30848101
// Primary template handles all types not supporting the operation.
template <typename, template <typename> class, typename = void_t<>>
struct detect : std::false_type {};
// Specialization recognizes/validates only types supporting the archetype.
template <typename T, template <typename> class Op>
struct detect<T, Op, void_t<Op<T>>> : std::true_type {};
// clone function
template <typename T> using fn_clone_t = decltype( std::declval<T>().clone() );
// has_clone
template <typename T> using has_clone = detect<T, fn_clone_t>;
This is an insanely complicated way of writing what should be a two-liner:
template<class T, class = void> struct has_clone : std::false_type {};
template<class T> struct has_clone<T, decltype(void(std::declval<T>().clone()))> : std::true_type {};
Consider this line of metaprogramming. (I'm not going to worry about what it does, for now.)
template <typename T, typename U, bool IsDefaultCopier>
struct slice_test : std::conditional<
std::is_same<T, U>::value // if U==T, no need to check for slicing
|| std::is_same<std::nullptr_t, U>::value // nullptr is fine
|| !IsDefaultCopier // user provided cloner, assume they're handling it
|| has_clone<typename std::remove_pointer<U>::type>::value // using default cloner, clone method must exist in U
, std::true_type
, std::false_type
>::type {};
Would you ever write
return ((t == u) || (nullptr == u) || !isdefaultcopier || has_clone(u))
? true : false;
? No? Then you shouldn't write the metaprogramming equivalent of return x ? true : false;
either. Just return the original boolean condition itself:
template <typename T, typename U, bool IsDefaultCopier>
struct slice_test : std::bool_constant<
std::is_same_v<T, U> // if U==T, no need to check for slicing
|| std::is_same_v<std::nullptr_t, U> // nullptr is fine
|| !IsDefaultCopier // user provided cloner, assume they're handling it
|| has_clone<std::remove_pointer_t<U>>::value // using default cloner, clone method must exist in U
> {};
(For C++11, use std::integral_constant<bool, XYZ>
in place of std::bool_constant<XYZ>
, and re-expand all my _t
s and _v
s. I shrank them here just to demonstrate how code should look if portability-to-C++11 is not a concern.)
struct
#ifdef _MSC_VER
// https://blogs.msdn.microsoft.com/vcblog/2016/03/30/optimizing-the-layout-of-empty-base-classes-in-vs2015-update-2-3/
__declspec( empty_bases ) // requires vs2015 update 2
#endif
ptr_data
I strongly recommend that you move the preprocessor logic to the top of the file, use it to set a macro, and use just the macro down here. It'll be easier to read and easier to maintain.
#ifdef _MSC_VER
#define USE_EMPTY_BASE_OPTIMIZATION __declspec(empty_bases)
#else
#define USE_EMPTY_BASE_OPTIMIZATION
#endif
// ...
struct USE_EMPTY_BASE_OPTIMIZATION ptr_data
: public std::unique_ptr<T, Deleter>
Inheriting (either publicly or privately) from standard-library types is never a good idea. I strongly recommend that you just implement the three special members (move-constructor, move-assignment, and destructor) yourself, rather than trying to reuse the STL's versions. ...Oh look, you do reimplement them, anyway!
ptr_data& operator=( const ptr_data& that ) {
if ( this == &that )
return *this;
*this = that.clone(); // R
return *this;
}
It's been a while since you wrote this code. How long will it take you to prove to yourself that the line marked // R
is not a recursive call to this operator=
? (Or is it? ;) Remember all of the behaviors of unique_ptr
!)
Speaking of which, inheriting from unique_ptr<T, Deleter>
is also kind of silly since unique_ptr<T, Deleter>
will have a data member of type Deleter::pointer
(which may be a fancy pointer type), whereas your clone()
methods seem to deal only in raw T*
. If you don't want the fancy-pointer behavior, you shouldn't drag it in. Yet another reason to implement the special members yourself instead of inheriting from unique_ptr
.
// tag dispatch on has_clone
return this->operator()( what
, typename std::conditional<detail::has_clone<T>::value, _clone, _copy>::type()
);
You seem to be a bit operator()
-happy in this code. There's no reason for this overload set to be named operator()
; I'd recommend something descriptive such as copy_or_clone
.
General rule of thumb: If you find yourself writing this->operator()(...)
instead of (*this)(...)
, then you are definitely doing it wrong. (And if you find yourself writing (*this)(...)
, you are probably doing it wrong, anyway.)
I also strongly recommend to avoid leading underscores. In this case, since they are tag types, _copy
should be spelled copy_tag
and _clone
should be spelled clone_tag
.
Did you consider using std::function<Base*(Base*)> m_clone
and std::function<void(Base*)> m_destroy
instead of rolling your own type-erasure? I mean, I love rolling my own type-erasure, and do it all the time; but if you're just practicing metaprogramming and want the shortest possible code, you might find that std::function
would be useful.
You could even use std::function<Base*(Base*, enum CloneOrDestroy)>
to wrap up both behaviors into a single function, to shrink the size of your value_ptr
object.
Did you consider what should happen if Base
's destructor is not virtual
? There is actually a type-trait for has_virtual_destructor<T>
, not that I would recommend using it (see my above advice about trusting your user). But consider how shared_ptr
works even with non-virtual destructors; should you do similarly?
This has been a long review already, and I didn't even really get to the meat of it. I would recommend simplifying the code a lot and re-posting for another round.
answered Nov 18 at 6:51
Quuxplusone
10.6k11854
10.6k11854
add a comment |
add a comment |
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f168920%2fvalue-ptrt-a-c11-header-only-deep-copying-smart-pointer-that-preserves-va%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
1
Shame nobody seems to have time to review this, it’s a very nice submission. Unfortunately I lack the time at the moment; just one thing I noticed; the identifier
_SMART_PTR_VALUE_PTR_
is reserved for the implementation, and is illegal in client code; the rule is: everything in the global namespace starting with__
(double underscore) or_
(single underscore) + uppercase letter is reserved.– Konrad Rudolph
Jul 14 '17 at 14:33
@KonradRudolph - thank you! I didn't know that about the leading underscore
– Tom
Jul 14 '17 at 16:33
Wow. I had never heard of
__declspec( empty_bases )
before now. That's something I'm gonna have to use.– Justin
Aug 2 '17 at 21:10
1
@Justin, there is standard solution coming, IIRC it is called
[[no_unique_address]]
.– Incomputable
Oct 30 at 5:33