Smart pointer like implementations

Question

For educational purposes I implemented standard library smart pointers like class templates. They are definitely not a full replacement for the library classes but I'd like to get some comments on the way I use templates and how I can do better. I'd especially like to get criticism about the code clarity and readability. Performance is not a big concern here but comments regarding poor design choices regarding it are welcome.
I have 3 classes:

1. Smart_pointer_base is an abstract base class for the other two.
2. Shrd_ptr is like std::shared_ptr
3. Unq_ptr is like std::unique_ptr

Those are in separate header files, and each has a .tpp file for definitions:

Smart_pointer_base.h

#ifndef BLOB_SMART_POINTER_BASE_H
#define BLOB_SMART_POINTER_BASE_H


// Superclass for Shrd_ptr and Unq_ptr
template <typename T>
class Smart_pointer_base {
public:
  explicit Smart_pointer_base(T*managed_): managed(managed_) { }
  Smart_pointer_base(): managed(nullptr) { }

  virtual T&operator*();
  virtual const T&operator*() const { return *managed; }

  virtual T*operator->();
  virtual const T*operator->() const { return managed; }

  virtual ~Smart_pointer_base() = default;

protected:

  /// `destruct` replaces the destructor
  /// derived classes may call it to
  /// deleted the `managed`
  inline virtual void destruct(){ delete managed; }

  // the pointer to the (hopefully) dynamically
  // allocated memory
  T* managed;
private:
};


// Definitions
#include "Smart_pointer_base.tpp"

#endif //BLOB_SMART_POINTER_BASE_H

Smart_pointer_base.tpp

#ifndef BLOB_SMART_POINTER_BASE_TPP
#define BLOB_SMART_POINTER_BASE_TPP


// Use the const version
template<typename T>
T &Smart_pointer_base<T>::operator*()
{
  const auto& res = const_cast<const Smart_pointer_base*>(this)->operator*();
  return const_cast<T&>(res);
}


// Use the const version
template<typename T>
T *Smart_pointer_base<T>::operator->()
{
  const auto* res = const_cast<const Smart_pointer_base*>(this)->operator->();
  return const_cast<T*>(res);
}

#endif // !BLOB_SMART_POINTER_BASE_TPP

Shrd_ptr.h

#ifndef BLOB_SHRD_PTR_H
#define BLOB_SHRD_PTR_H

#include <cstddef>
#include "Smart_pointer_base.h"
#include <functional>
#include <utility>

template <typename Y> const Y safe_increment(Y*);
template <typename Y> const Y safe_decrement(Y*);

template <typename T>
class Shrd_ptr: public Smart_pointer_base<T> {
  typedef std::size_t size_type;
  typedef std::function<void(const T*)> destructor_type;
public:
  explicit Shrd_ptr(T* managed_, destructor_type destructor_ = nullptr)
    : Smart_pointer_base<T>(managed_)
    , user_count(new size_type(1))
    , destructor(std::move(destructor_))
  { }

  Shrd_ptr();

  Shrd_ptr(const Shrd_ptr&);

  Shrd_ptr&operator=(const Shrd_ptr&);

  ~Shrd_ptr() override;

  void reset() noexcept { Shrd_ptr().swap(*this);}

  template< class Y >
  void reset( Y* ptr ) { Shrd_ptr<T>(ptr).swap(*this); }

  template< class Y, class Deleter>
  void reset( Y* ptr, Deleter d) { Shrd_ptr<T>(ptr, d).swap(*this); }

  void swap(Shrd_ptr& r ) noexcept;
private:
  void destruct() override;
  void copy(const Shrd_ptr&);

  destructor_type destructor;
  mutable size_type* user_count;


  // friends
  friend const T safe_increment<T>(T*);
  friend const T safe_decrement<T>(T*);
};



// Definitions
#include "Shrd_ptr.tpp"

#endif //BLOB_SHRD_PTR_H

Shrd_ptr.tpp

#ifndef BLOB_SHRD_PTR_TPP
#define BLOB_SHRD_PTR_TPP

template <typename T> class Smart_pointer_base;

template <typename T> class Shrd_ptr;

template<typename T>
Shrd_ptr<T>::~Shrd_ptr()
{
  destruct();
}

template<typename T>
Shrd_ptr<T>::Shrd_ptr(const Shrd_ptr &rhs)
  : Smart_pointer_base<T>(rhs.managed)
  , user_count(rhs.user_count)
  , destructor(rhs.destructor)
{
    // increase user_count by one
    // to denote added user
    safe_increment(user_count);
}

template<typename T>
Shrd_ptr<T> &Shrd_ptr<T>::operator=(const Shrd_ptr &rhs)
{
  // Check equality by comparing managed pointers
  if (this->managed != rhs.managed) {
    destruct();
    copy(rhs);
  }
  return *this;
}

template<typename T>
void Shrd_ptr<T>::destruct()
{
  // if count reaches zero
  // No other pointer points
  // to this managed
  if (safe_decrement(user_count) == 0) {
    delete user_count;
    destructor ? destructor(this->managed) : Smart_pointer_base<T>::destruct();
  }
}


template<typename T>
void Shrd_ptr<T>::copy(const Shrd_ptr &rhs)
{
  // this function assumes that all
  // necessary the destruction has been
  // carried out

  this->managed = rhs.managed;

  safe_increment(rhs.user_count);

  user_count = rhs.user_count;
  destructor = rhs.destructor;
}


template<typename T>
Shrd_ptr<T>::Shrd_ptr()
  : Smart_pointer_base<T>(nullptr)
  , user_count(nullptr)
  , destructor(nullptr)
{ }


template<typename T>
void Shrd_ptr<T>::swap(Shrd_ptr &r) noexcept
{
  using std::swap;
  swap(this->managed, r.managed);
  swap(user_count, r.user_count);
  swap(destructor, r.destructor);
}

// increment whatever `ptr` points to
// only if it is `ptr != nullptr`
// returns 0 if `ptr == nullptr`
template<typename T>
inline const T safe_increment(T *ptr)
{
  if (ptr)
    return ++*ptr;
  return T();
}

// decrement whatever `ptr` points to
// only if it is `ptr != nullptr`
// returns 0 if `ptr == nullptr`
template<typename T>
inline const T safe_decrement(T *ptr)
{
  if (ptr)
    return --*ptr;
  return T();
}


#endif // !BLOB_SHRD_PTR_TPP

Unq_ptr.h

#ifndef BLOB_UNQ_PTR_H
#define BLOB_UNQ_PTR_H

#include <functional>
#include "Smart_pointer_base.h"

template <typename T, typename destructor_type = std::function<void(const T*)>>
class Unq_ptr: public Smart_pointer_base<T> {
public:
  explicit Unq_ptr(T*, destructor_type = [](const T*t){ delete t; });
  Unq_ptr(Unq_ptr&&) noexcept;
  Unq_ptr&operator=(Unq_ptr&&) noexcept;

  Unq_ptr(const Unq_ptr&) = delete;
  Unq_ptr&operator=(const Unq_ptr&) = delete;
  ~Unq_ptr() override;
private:
  destructor_type destructor;
  void move(Unq_ptr&&) noexcept;
  void destruct() override;
};

// Definitions
#include "Unq_ptr.tpp"


#endif //BLOB_UNQ_PTR_H

Unq_ptr.tpp

#ifndef BLOB_UNQ_PTR_TPP
#define BLOB_UNQ_PTR_TPP

#include "Unq_ptr.h"

template<typename T, typename destructor_type> class Unq_ptr;

template<typename T, typename destructor_type>
Unq_ptr<T, destructor_type>::Unq_ptr(T * managed_, destructor_type destructor_)
    : Smart_pointer_base<T>(managed_)
    , destructor(destructor_)
{ }

template<typename T, typename destructor_type>
void Unq_ptr<T, destructor_type>::move(Unq_ptr &&rhs) noexcept
{
  this->managed = rhs.managed;
  rhs.managed = nullptr;

  // No need to set `rhs.destructor` to `nullptr`
  destructor = rhs.destructor;
}


template<typename T, typename destructor_type>
void Unq_ptr<T, destructor_type>::destruct()
{
  destructor(this->managed);
}

template<typename T, typename destructor_type>
Unq_ptr<T, destructor_type>::Unq_ptr(Unq_ptr &&rhs) noexcept
{
  move(rhs);
  destruct();
}

template<typename T, typename destructor_type>
Unq_ptr<T, destructor_type> &Unq_ptr<T, destructor_type>::operator=(Unq_ptr &&rhs) noexcept
{
  // No need to check for self assignment
  // since we only take an r-value assignment
  move(rhs);
  destructor();
  return *this;
}

template<typename T, typename destructor_type>
Unq_ptr<T, destructor_type>::~Unq_ptr()
{
  destruct();
}

#endif // !BLOB_UNQ_PTR_TPP

Besides those, I have a source file to test those headers:

test.cpp

#include <vector>
#include <iostream>
#include "Smart_pointer_base.h"
#include "Shrd_ptr.h"
#include "Unq_ptr.h"
#include <functional>

using std::vector;
using std::cout;
using std::endl;
using std::function;

using ivec = vector<int>;

// these `get_shared` and `get_unique` functions are
// used only to get smart pointers to dynamically
// allocated objects
Shrd_ptr<ivec> get_shared()
{ return Shrd_ptr<ivec>(new ivec{1, 3, 5, 7, 9}, [](const ivec*p){ delete p; cout << "Shared Deleted!\n";}); }

Unq_ptr<ivec> get_unique()
{
  return Unq_ptr<ivec, function<void(const ivec*)>>
    (new ivec{0, 2, 4, 6, 8}, [](const ivec*p){ delete p; cout << "Unique Deleted!\n"; } );
}



int main()
{
  auto e_shr = get_shared();
  auto b_shr = e_shr;

  e_shr.reset();
  auto s_shr = b_shr;

  b_shr.reset();

  for (const auto& elm : *s_shr) {
    cout << elm << endl;
  }

  auto e_unq = get_unique();

  for (const auto& elm : *e_unq) {
    cout << elm << endl;
  }
  return 0;
}

Answer 1

Common biggest mistake.

The largest common problem is not ensuring ownership is taken during construction. The problem is that if the constructor does not complete (ie an exception is thrown out of the constructor) then the destructor will never be called.

But so what you ask. Well if you blow up during construction you have definitely leaked the pointer you just promised to manage!

  explicit Shrd_ptr(T* managed_, destructor_type destructor_ = nullptr)
    : Smart_pointer_base<T>(managed_)
    , user_count(new size_type(1))
    , destructor(std::move(destructor_))
  { }

It looks so simple. How can that not complete? Well the you have a call to new in there. It is not exception safe and can throw. So what can you do about it?

Two options. 1) Use a non throwing new: new (std::nothrow) size_type(1) 2) catch exceptions in the constructor using Function try blocks.

# Option 1
  explicit Shrd_ptr(T* managed_, destructor_type destructor_ = nullptr)
    : Smart_pointer_base<T>(managed_)
    , user_count(new (std::nothrow) size_type(1))
    , destructor(std::move(destructor_))
  {
      if (user_count == nullptr) {
          destructor(managed_);
          throw std::bad_alloc("Failed");
      }
  }    

# Option 2
  explicit Shrd_ptr(T* managed_, destructor_type destructor_ = nullptr)
    try
        : Smart_pointer_base<T>(managed_)
        , user_count(new size_type(1))
        , destructor(std::move(destructor_))
    {}
    catch(...)
    {
         destructor_(managed_);
         throw;   // re-throw current exception
    }

Virtual Functions

Calling virtual functions in the constructor or destructor is UB. Because it is implementation defined how virtual functions are implemented the standard has put restrictions on when they can be used. The can not be used during construction or destruction.

Shrd_ptr<T>::~Shrd_ptr()
{
  destruct();  // This is UB
}

Since each shared pointer is unique. Just put the code you have in destruct() in the destructor for that type of object. That will do exactly what you expect:

Shrd_ptr<T>::~Shrd_ptr()
{
    if (safe_decrement(user_count) == 0) {
        delete user_count;
        destructor ? destructor(this->managed) : Smart_pointer_base<T>::destruct();
    }
}

Unq_ptr<T, destructor_type>::~Unq_ptr()
{
    destructor(this->managed);
}

Optimization

You create an object to hold the count.

In the shared pointer you keep a pointer to this count object and a destructor object. When you copy/move both these values need to be updated. Why not modify the count object to store all the accessory information about the shared pointer (both count and destructor object). That way when you copy/move you just need to move one pointer (in addition to the data).

Missing Functionality

When you have de-reference and de-refrence method functions you should really have a way to check that the object can be de-referenced. Otherwise you have to hope it does not blow up when you use these methods.

Basically you need some way to check the pointer you are holding is not a nullptr.

 explicit operator bool() const {return managed;}

Inconsistent behavior

If you explicitly construct a shared pointer with a nullptr and you create a shared pointer using the default constructor (this internally it is nullptr) you have different internal structures (one has a user count the other does not).

Shrd_ptr<int>     data1(nullptr);
Shrd_ptr<int>     data2;

I can not see any bugs with this. But as your class gets bigger having this inconsistency may cause unforeseen issues. No matter how the object is constructed it should have the same internal structure for the same data.

No nullptr constructor.

You don't have a constructor for explicitly taking nullptr as a parameter.

But you say nullptr binds to any pointer type. Yes. But its not about the normal situation.

This is a situation where you are passing a nullptr to function/method. If the function takes an r-value ref we can not construct the object (as the constructor is explicit) so you need to fully qualify the nullptr to make the call.

void workWithSP(Shrd_ptr<int>&& sp); // Some definition.


int main()
{
    workWithSP(nullptr);                // Does not work.

    workWithSP(Shrd_ptr<int>(nullptr)); // Works but seems verbose.
}

Simply add a nullptr_t constructor.

Shrd_ptr::Shrd_ptr(std::nullptr_t): ....

Missing Functionality

Assigning derived types.

 class X {};
 class Y: public X {};

 Shrd_ptr<Y>    data(new Y);
 Shrd_ptr<X>    dataNew(std::move(data));  // Why does that not work?