A lightweight version of std::shared_ptr<T>

Question

I wrote a simple lighter version of std::shared_ptr<T> called res_ptr<T>.

std::shared_ptr<T> is somewhat heavy due to all the nuances of its control block: support for safe deleting without virtual destructor, support for std::weak_ptr<T> (I am not certain on all details of the implementation... but I believe that it either requires an extra allocation for the control block, or has complicated logic on when to delete...).

The res_ptr<T> requires T to inherit from class resource that manages the reference counter and deletes the instance upon destruction once the counter reaches zero requires. Yes, it doesn't support array version res_ptr<T[]> and I don't think that it should but I might wrong.

res_version<T> is a helper class that attaches resource class to classes that do not inherit from resource that shouldn't change anything besides that.

Besides that, it is supposed to have fairly same interface as std::shared_ptr<T> and std::unique_ptr<T>. I know it currently lacks option for custom deleter but I don't think that I'll ever need it so I don't intend to implement it.

The implementation I have is written on Visual Studio and isn't fully portable. Though, honestly, this class is going to be the least of my worries once I need portability for the codebase. Though, if it is something else besides modifying #pragma once or issues with initialization of std::atomic I'd like to know.

I'd like to know if there are any issues or downsides that I failed to consider as well as general code review.

#pragma once
#include <atomic>

class resource
{
public:
    virtual ~resource()         = default;

    resource()                  = default;

    // moving / copying does not alter the reference counter
    resource(resource&&)                    noexcept : resource() {};
    resource(const resource&)               noexcept : resource() {};
    resource& operator = (resource&&)       noexcept {};
    resource& operator = (const resource&)  noexcept {};

    void    add_ref()               const   noexcept
    {
        m_refcount.fetch_add(1, std::memory_order_relaxed);
    }

    int     reduce_ref()            const   noexcept
    {
        return m_refcount.fetch_sub(1, std::memory_order_relaxed)-1;
    }

    int     count()                 const   noexcept
    {
        return m_refcount.load(std::memory_order_relaxed);
    }

private:
    mutable std::atomic<int>    m_refcount = 0;
};

template<typename T>
class res_version :
    public T, public resource
{
public:
    template<typename... Args>
    res_version(Args... args):
        T(std::forward<Args>(args)...)
    {};
};

template<typename PResource, typename Enable = void>
class res_ptr;

template<typename PResource>
class res_ptr<PResource, typename std::enable_if_t<std::is_base_of_v<resource, PResource>>>
{
public:
    template<typename PResourceDerived, typename E>
    friend class res_ptr;

    constexpr   res_ptr()           noexcept = default;
    constexpr   res_ptr(nullptr_t)  noexcept {};

    template<typename PResourceDerived, std::enable_if_t<std::is_base_of_v<PResource, PResourceDerived>, int> = 0>
    explicit    res_ptr(PResourceDerived* ptr) : m_ptr(static_cast<PResource*>(ptr))
    {
        if(m_ptr) m_ptr->add_ref();
    };

    ~res_ptr()
    {
        release();
    }

    // copy ctor
    res_ptr(const   res_ptr& ptr) noexcept :
        m_ptr(ptr.get())
    {
        if (m_ptr) m_ptr->add_ref();
    };

    // copy ctor cast
    template<typename PResourceDerived, std::enable_if_t<std::is_base_of_v<PResource, PResourceDerived> && !std::is_same_v<PResource, PResourceDerived>,int> = 0>
    res_ptr(            const   res_ptr<PResourceDerived> & ptr) noexcept :
        m_ptr(static_cast<PResource*>(ptr.get()))
    {
        if (m_ptr) m_ptr->add_ref();
    };

    // move ctor
    res_ptr(                    res_ptr&& ptr) noexcept :
        m_ptr(std::exchange(ptr.m_ptr, nullptr))
    {};

    // move ctor cast
    template<typename PResourceDerived, std::enable_if_t<std::is_base_of_v<PResource, PResourceDerived> && !std::is_same_v<PResource, PResourceDerived>,int> = 0>
    res_ptr(                    res_ptr<PResourceDerived> &&    ptr) noexcept :
        m_ptr(static_cast<PResource*>(std::exchange(ptr.m_ptr, nullptr)))
    {};

    // copy
    res_ptr& operator = (const  res_ptr&        other) noexcept
    {
        if (this != &other)
        {
            release();
            m_ptr = other.m_ptr;

            if (m_ptr) m_ptr->add_ref();
        }

        return *this;
    }

    // move
    res_ptr& operator = (       res_ptr&&       other) noexcept
    {
        if (this != &other)
        {
            release();
            m_ptr = std::exchange(other.m_ptr,nullptr);
        }

        return *this;
    }

    // copy cast
    template<typename PResourceDerived, std::enable_if_t<std::is_base_of_v<PResource, PResourceDerived> && !std::is_same_v<PResource, PResourceDerived>, int> = 0>
    res_ptr& operator = (const  res_ptr<PResourceDerived>&      other) noexcept
    {
        release();
        m_ptr = static_cast<PResource*>(other.m_ptr);

        if (m_ptr) m_ptr->add_ref();

        return *this;
    }

    // move cast
    template<typename PResourceDerived, std::enable_if_t<std::is_base_of_v<PResource, PResourceDerived> && !std::is_same_v<PResource, PResourceDerived>, int> = 0>
    res_ptr& operator = (       res_ptr<PResourceDerived>&& other) noexcept
    {
        release();
        m_ptr = static_cast<PResource*>(std::exchange(other.m_ptr,nullptr));

        return *this;
    }

    PResource* operator ->  ()  const noexcept
    {
        return m_ptr;
    }
    PResource& operator *   ()  const noexcept
    {
        return *m_ptr;
    }
    PResource* get()            const noexcept
    {
        return m_ptr;
    }
    operator bool   ()          const noexcept
    {
        return m_ptr != nullptr;
    }

    void release()
    {
        if (m_ptr && (m_ptr->reduce_ref() == 0))
        {
            delete m_ptr;
        }
    }

    template<typename PResource>
    bool operator == (const res_ptr<PResource>& other) noexcept
    {
        return m_ptr == other.m_ptr;
    }

    template<typename PResource>
    bool operator != (const res_ptr<PResource>& other) noexcept
    {
        return m_ptr != other.m_ptr;
    }

private:
    PResource* m_ptr = nullptr;
};

template<typename PResource, typename... Args>
res_ptr<PResource>          make_resource(Args&& ... args)
{
    return res_ptr<PResource>(new PResource(std::forward<Args>(args)...));
}

template<typename PResourceDerived, typename PResourceBase>
res_ptr<PResourceDerived>   resource_dynamic_cast(const res_ptr<PResourceBase>& uPtr) noexcept
{
    PResourceDerived* ptr = dynamic_cast<PResourceDerived*>(uPtr.get());

    return res_ptr<PResourceDerived>(ptr);
}

Answer 1

Missing headers

• <cstddef> for std::nullptr_t
• <type_traits> for std::is_base_of_v, std::is_same_v, std::enable_if_t
• <utility> for std::exchange, std::forward

Layout

The code is hard to read, with long lines and huge blocks of spaces (perhaps a misguided attempt to align keywords?). Stick to a conventional layout and it will be much easier to read.

Fix the errors and warnings

These should need no further explanation (except perhaps the one caused by misspelling std::nullptr_t):

231599.cpp: In member function ‘resource& resource::operator=(resource&&)’:
231599.cpp:13:55: warning: no return statement in function returning non-void [-Wreturn-type]
   13 |     resource& operator = (resource&&)       noexcept {};
      |                                                       ^
      |                                                       return *this;
231599.cpp: In member function ‘resource& resource::operator=(const resource&)’:
231599.cpp:14:55: warning: no return statement in function returning non-void [-Wreturn-type]
   14 |     resource& operator = (const resource&)  noexcept {};
      |                                                       ^
      |                                                       return *this;
231599.cpp: At global scope:
231599.cpp:43:7: warning: extra ‘;’ [-Wpedantic]
   43 |     {};
      |       ^
      |       -
231599.cpp:57:5: error: non-static data member ‘nullptr_t’ declared ‘constexpr’
   57 |     constexpr   res_ptr(nullptr_t)  noexcept {};
      |     ^~~~~~~~~
231599.cpp:57:34: error: expected ‘;’ at end of member declaration
   57 |     constexpr   res_ptr(nullptr_t)  noexcept {};
      |                                  ^
      |                                   ;
231599.cpp:57:37: error: expected unqualified-id before ‘noexcept’
   57 |     constexpr   res_ptr(nullptr_t)  noexcept {};
      |                                     ^~~~~~~~
231599.cpp:63:6: warning: extra ‘;’ [-Wpedantic]
   63 |     };
      |      ^
      |      -
231599.cpp:83:6: warning: extra ‘;’ [-Wpedantic]
   83 |     };
      |      ^
      |      -
231599.cpp:94:7: warning: extra ‘;’ [-Wpedantic]
   94 |     {};
      |       ^
      |       -
231599.cpp:169:14: error: declaration of template parameter ‘PResource’ shadows template parameter
  169 |     template<typename PResource>
      |              ^~~~~~~~
231599.cpp:49:10: note: template parameter ‘PResource’ declared here
   49 | template<typename PResource>
      |          ^~~~~~~~
231599.cpp:175:14: error: declaration of template parameter ‘PResource’ shadows template parameter
  175 |     template<typename PResource>
      |              ^~~~~~~~
231599.cpp:49:10: note: template parameter ‘PResource’ declared here
   49 | template<typename PResource>
      |          ^~~~~~~~
231599.cpp: In constructor ‘res_ptr<PResource, typename std::enable_if<is_base_of_v<resource, PResource>, void>::type>::res_ptr(res_ptr<PResource, typename std::enable_if<is_base_of_v<resource, PResource>, void>::type>&&)’:
231599.cpp:87:20: error: ‘exchange’ is not a member of ‘std’
   87 |         m_ptr(std::exchange(ptr.m_ptr, nullptr))
      |                    ^~~~~~~~
231599.cpp: In constructor ‘res_ptr<PResource, typename std::enable_if<is_base_of_v<resource, PResource>, void>::type>::res_ptr(res_ptr<PResourceDerived>&&)’:
231599.cpp:93:44: error: ‘exchange’ is not a member of ‘std’
   93 |         m_ptr(static_cast<PResource*>(std::exchange(ptr.m_ptr, nullptr)))
      |                                            ^~~~~~~~
231599.cpp: In member function ‘res_ptr<PResource, typename std::enable_if<is_base_of_v<resource, PResource>, void>::type>& res_ptr<PResource, typename std::enable_if<is_base_of_v<resource, PResource>, void>::type>::operator=(res_ptr<PResource, typename std::enable_if<is_base_of_v<resource, PResource>, void>::type>&&)’:
231599.cpp:116:26: error: ‘exchange’ is not a member of ‘std’
  116 |             m_ptr = std::exchange(other.m_ptr,nullptr);
      |                          ^~~~~~~~
231599.cpp: In member function ‘res_ptr<PResource, typename std::enable_if<is_base_of_v<resource, PResource>, void>::type>& res_ptr<PResource, typename std::enable_if<is_base_of_v<resource, PResource>, void>::type>::operator=(res_ptr<PResourceDerived>&&)’:
231599.cpp:139:46: error: ‘exchange’ is not a member of ‘std’
  139 |         m_ptr = static_cast<PResource*>(std::exchange(other.m_ptr,nullptr));
      |                                              ^~~~~~~~
231599.cpp: In function ‘int main()’:
231599.cpp:207:34: error: invalid use of incomplete type ‘class res_ptr<int, void>’
  207 |     auto a = make_resource<int>(5);
      |                                  ^
231599.cpp:47:7: note: declaration of ‘class res_ptr<int, void>’
   47 | class res_ptr;
      |       ^~~~~~~
231599.cpp: In instantiation of ‘res_ptr<PResource> make_resource(Args&& ...) [with PResource = int; Args = {int}]’:
231599.cpp:207:34:   required from here
231599.cpp:186:29: error: return type ‘class res_ptr<int, void>’ is incomplete
  186 | res_ptr<PResource>          make_resource(Args&& ... args)
      |                             ^~~~~~~~~~~~~
231599.cpp:188:12: error: invalid use of incomplete type ‘class res_ptr<int, void>’
  188 |     return res_ptr<PResource>(new PResource(std::forward<Args>(args)...));
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
231599.cpp:47:7: note: declaration of ‘class res_ptr<int, void>’
   47 | class res_ptr;
      |       ^~~~~~~

Usage guidance

My first simple test, expecting a similar interface to std::make_shared(), failed:

auto a = make_resource<int>(5);

So I tried the next most obvious course:

auto a = make_resource<res_version<int>>(5);

This also fails, due to attempting to inherit from a non-struct type.

It seems I need to create a struct (with a constructor) even for something this simple! Much less friendly than the standard shared pointer.

Questionable choices

Why use int to count references? An unsigned type would be more appropriate.

We should check for overflow before incrementing the counter, and throw an exception if that would happen, otherwise we could delete objects still in use. I don't think that's possible without changing away from std::memory_order_relaxed.

The res_version adapter should virtually inherit resource, to avoid the diamond problem when subclasses inherit from more than one of these.

I'd prefer the res_version constructor to include an initializer for the resource base, rather than omitting it. The T() constructor is likely to be tricky here, as it makes it hard for users to select the T{} constructor - important for classes such as std::vector.

The statement if (m_ptr) m_ptr->add_ref(); is used many times - worth encapsulating in a member function (any decent compiler will inline it).

std::is_base_of_v<PResource, PResourceDerived> && !std::is_same_v<PResource, PResourceDerived> is used many times - encapsulate that too (is_derived_from<>, perhaps?).

We haven't implemented swap(), so std::swap() will fall back to copying via a temporary, which implies needless updates to the count.

res_ptr::operator=(res_ptr&&) can be implemented in terms of swap() (if we write that), instead of releasing first.

Answer 2

The only thing I see that is a mistake is the bool operator. Instead of:

operator bool   ()          const noexcept

You probably want:

explicit operator bool   ()          const noexcept

The explicit prevents the compiler from being able to automatically convert your object to bool as part of a comparison.

res_ptr<X>     data = getResPtr();

if (true == data) {
    // Not sue you want that auto conversion.
    // The standard shared_ptr does not want this implicit conversion
    // I am sure there are other cases where it can be out-converted where
    // you don't want that to happen.

         doSomeWorkWithBool(data); // If that function takes a bool
                                   // it auto converts before the call.
                                   // auto conversions are not usually what
                                   // you want.
}

// Note:
if (data) {
    // This still works as expected with the explicit
    // It is designed so that in a bool context like if()/while()/for() etc
    // the explicit conversion to bool is applied but in other contexts it
    // does not work any you need an explicit cast

    doSomeWorkWithBool(static_cast<bool>(data)); // Need an explict cast here
                                                 // if you use the `explicit`
                                                 // in the bool operator.
}