C++ UniquePtr Implementation

Question

Took a shot at implementing std::unique_ptr.

Code:

#include <memory>
#include <stdexcept>

template<typename T>
struct DefaultDeleter {
    void operator()(T* ptr) const  {
        delete ptr;
    }
};

template <typename T>
struct DefaultDeleter<T[]> {
    void operator()(T* ptr) const {
        delete[] ptr;
    }
};

template<typename T, typename Deleter = DefaultDeleter<T>>
class UniquePtr {
public:
    using pointer = T*;
    using element_type = T;
    using deleter_type = Deleter;

    constexpr UniquePtr() noexcept : m_ptr{nullptr} {}
    explicit UniquePtr (pointer p) noexcept : m_ptr{p} {}
    explicit UniquePtr (pointer p, const deleter_type& deleter) :
        m_ptr{p}, 
        m_deleter{deleter} 
    {}

    UniquePtr(const UniquePtr& other) = delete;
    UniquePtr& operator=(const UniquePtr& other) = delete; 

    UniquePtr(UniquePtr&& other) noexcept :
        m_ptr{other.release()}, 
        m_deleter{std::move(other.m_deleter)} 
    {}

    // Added this to allow for runtime polymorphism, but I didn't 
    // see such a constructor on cppreference. Am I just missing something
    // or should this be handled another way? 
    template<typename U, 
        std::enable_if_t<std::is_convertible_v<U*, T*>, bool> = false>
    UniquePtr(UniquePtr<U>&& other) noexcept :
        m_ptr{other.release()} 
    {}

    UniquePtr& operator=(UniquePtr&& other) noexcept {
        if (this != &other) {
            m_deleter(m_ptr);
            m_ptr = other.m_ptr;
            other.m_ptr = nullptr;
            m_deleter = std::move(other.m_deleter);
        }

        return *this;
    }

    pointer release() noexcept {
        pointer result = m_ptr;
        m_ptr = nullptr;
        return result;
    }

    void reset(pointer ptr = pointer()) noexcept {
        pointer old_ptr = m_ptr;
        m_ptr = ptr;
        if (old_ptr) {
            m_deleter(old_ptr);
        }
    }

    void swap(UniquePtr& other) noexcept {
        std::swap(m_ptr, other.ptr_);
        std::swap(m_deleter, other.m_deleter);
    }

    pointer get() const noexcept {
        return m_ptr;
    }

    explicit operator bool() const noexcept {
        return get() != nullptr;
    }

    std::add_lvalue_reference_t<element_type> operator* () const 
        noexcept(noexcept(*std::declval<pointer>())) {
        return *m_ptr;
    }

    pointer operator->() const noexcept {
        return m_ptr;
    }

    ~UniquePtr() {
        m_deleter(m_ptr);
    }
private:
    pointer m_ptr;
    deleter_type m_deleter;
};


template<typename T, typename... Args>
UniquePtr<T> makeUnique(Args&&... args) {
    return UniquePtr<T>(new T(std::forward<Args>(args)...));
}

Some tests, mostly attempting to show the example functionality on cppreference.com works with my implementation. Plus a couple extra tests to fill in gaps/test things a bit more precisely.

#include <cassert>
#include <fstream>
#include <iostream>
#include "UniquePtr.h"

/* Test setup code (adapted from cppreference.com) */

// helper class for runtime polymorphism demo below
struct B
{
    virtual ~B() = default; 
    virtual int val() const {return 0;}
};
 
struct D : B
{
    D() { }
    ~D() {}
 
    int val() const override {
        // dummy value to demonstrate dereferencing
        return 2;
    }
};
 
// a function consuming a unique_ptr can take it by value or by rvalue reference
UniquePtr<D> pass_through(UniquePtr<D> p)
{
    p->val();
    return p;
}
 
// helper function for the custom deleter demo below
void close_file(std::FILE* fp)
{
    std::fclose(fp);
}

struct Node
{
    int data;
    UniquePtr<Node> next;
};
// unique_ptr-based linked list demo
struct List
{
    UniquePtr<Node> head;
 
    ~List()
    {
        // destroy list nodes sequentially in a loop, the default destructor
        // would have invoked its `next`'s destructor recursively, which would
        // cause stack overflow for sufficiently large lists.
        while (head)
        {
            
            // std::cout << "before first move \n";
            auto next = std::move(head->next);
            // std::cout << "before second move \n";
            head = std::move(next);
        }
    }
 
    void push(int data)
    {
        head = UniquePtr<Node>(new Node{data, std::move(head)});
    }
};

/* Beginning of tests */

void testOwnershipTransfer() {
    UniquePtr<D> p = makeUnique<D>();
    UniquePtr<D> q = pass_through(std::move(p));
    assert(!p);
}

void testNullUniquePtr() {
    UniquePtr<D> p;
} // this function should exit successfully 

void testDererference() {
    UniquePtr<D> p = makeUnique<D>();
    D obj = *p;
    assert(obj.val() == 2);
}

void testBool() {
    UniquePtr<D> p;
    assert(!p);

    p = makeUnique<D>();
    assert(p);

    p.reset();
    assert(!p);
}

void testRuntimePolymorphism() {
    UniquePtr<B> p = makeUnique<D>();
    assert(p -> val() == 2);  
}

void testFileDeletion() {
    std::ofstream("test.txt") << 'x'; // prepare the file to read
    {
        using unique_file_t = UniquePtr<std::FILE, decltype(&close_file)>;
        unique_file_t fp(std::fopen("test.txt", "r"), &close_file);
        if (fp) {
            assert(char(std::fgetc(fp.get())) == 'x');
        }
    } // `close_file()` called here (if `fp` is not null)
}

void testExceptionSafety() {
    try
    {
        UniquePtr<D, void(*)(D*)> p(new D, [](D* ptr)
        {
            delete ptr;
        });
 
        throw std::runtime_error(""); // `p` would leak here if it were a plain pointer
    }
    catch (const std::exception&) { std::cout << "Caught exception\n"; }
}

void testLinkedList() {
    List wall;
    for (int i = 0; i != 1'000'000; ++i)
        wall.push(i);
} // function should exit successfully 


int main()
{
    testOwnershipTransfer();
    testNullUniquePtr();
    testDererference();
    testBool();
    testRuntimePolymorphism();
    testFileDeletion();
    testExceptionSafety();
    testLinkedList();
}

This omits the array specialization, because when taking a peek at the GCC implementation it seems to be mostly a duplication of the class with some variations here and there to support arrays. Is this how it needs to be done, or is it possible to reuse the existing code somehow and write a small, concise class for the array specialization?

Thanks in advance to anyone who can take a look!

Answer 1

The converting constructor you added is part of the interface of std::unique_ptr. It is number (6) of the list on cppreference. However you should change yours to also accept UniquePtr's with a different deleter. But you have to be somewhat pedantic to exhibit the required behaviour if Deleter is a reference type.

// To access private members of UniquePtr's with different template parameters
template <typename, typename> friend class UniquePtr;

// Converting constructors
// Version 1 for non-reference deleter types
template<typename U, typename E,
    std::enable_if_t<std::is_convertible_v<U*, T*>, bool> = false,
    std::enable_if_t<!std::is_reference_v<Deleter> && std::is_convertible_v<E, Deleter>,
                     bool> = false>
UniquePtr(UniquePtr<U, E>&& other) noexcept :
    m_ptr{other.release()}, m_deleter{std::move(other.m_deleter)} {}

// Converting constructors
// Version 2 for reference deleter types
template<typename U, typename E,
    std::enable_if_t<std::is_convertible_v<U*, T*>, bool> = false,
    std::enable_if_t<std::is_reference_v<Deleter> && std::is_same_v<E, Deleter>,
                     bool> = false>
UniquePtr(UniquePtr<U, E>&& other) noexcept :
    m_ptr{other.release()}, m_deleter{other.m_deleter} {}

However for the new converting constructors to work, we need to specify a conversion for the different deleter types involved:

DefaultDeleter() = default;
    
template <typename U,
          std::enable_if_t<std::is_convertible_v<U*, T*>, bool> = true>
DefaultDeleter(DefaultDeleter<U> const&) {}

---

Probably most of the time Deleter will be DefaultDeleter<T>. DefaultDeleter<T> is an empty type. But since it's a member of UniquePtr, C++ requires it to have a unique memory address. Because of this your UniquePtr will have size 16 instead of size 8 (on 64-bit systems), with the second 8 bytes being wasted space. To solve this, you could add the [[no_unique_address]] attribute. Unfortunately, microsofts C++ compiler does not honour this attribute. Another solution is to exploit empty base class optimization, which all major C++ compilers support. Make a private base class of UniquePtr, that has the deleter as a member or as a base class, depending on whether Deleter is a class type or not.

template <typename Deleter, bool = std::is_class_v<Deleter> && !std::is_final_v<Deleter>>
struct DeleterHolder: private Deleter {
    // Use perfect forwarding for construction with class types
    template <typename E>
    DeleterHolder(E&& e): Deleter(std::forward<E>(e)) {}

    Deleter& get_deleter() noexcept {
        return static_cast<Deleter&>(*this);
    }

    const Deleter& get_deleter() const noexcept {
        return static_cast<Deleter const&>(*this);
    }
};

template <typename Deleter>
struct DeleterHolder<Deleter, false> {
    // Simply pass pointers and references 'by value'
    DeleterHolder(Deleter d): m_deleter(d) {}

    Deleter& get_deleter() noexcept {
        return this->m_deleter;
    }

    const Deleter& get_deleter() const noexcept {
        return this->m_deleter;
    }

private:
    Deleter m_deleter;
};

template<typename T, typename Deleter = DefaultDeleter<T>>
class UniquePtr: DeleterHolder<Deleter> { 
public:
    using DeleterHolder<Deleter>::get_deleter;

    // Rest of the implementation...
};

Now you have to go through all the places where you use the deleter and update the code there to use get_deleter and the constructor of DeleterHolder.

We do not unconditionally inherit from Deleter, because it might be a function type, which you cannot inherit from.

Thanks to @Deduplicator for pointing out an issue in the previous version of the code here :-)

---

Otherwise the implementation looks fine from what I can see :-)

It is great that you added tests. But you should add more tests that directly verify that the contracts of the smart pointer are fulfilled. For example, check that objects always get deleted when the pointer goes out of scope, check that no double deletion happens etc. Also you might want to consider to use a test framework such as Catch2 or google test.

Answer 2

Use the Swap Idiom to Move Instances

Among other benefits, swaps can be thread-safe atomic operations that never throw exceptions.

About That Converting Constructor

(Thanks to @chrysante for pointing out a serious oversight in my original answer.)

One important piece of functionality here is the ability to do things like:

 std::unique_ptr<Base> foo = std::make_unique<Derived>(bar, baz);

You implement this with the converting constructor

    // Added this to allow for runtime polymorphism, but I didn't 
    // see such a constructor on cppreference. Am I just missing something
    // or should this be handled another way? 
    template<typename U, 
        std::enable_if_t<std::is_convertible_v<U*, T*>, bool> = false>
    UniquePtr(UniquePtr<U>&& other) noexcept :
        m_ptr{other.release()} 
    {}

First, in a reinventing-the-wheel project, you get to use requires from C++20, which is much nicer than std::enable_if. So I recommend that.

Second, this implementation will break if the pointer conversion changes the bits of the pointer, even if it is from a derived class to a base class with a virtual destructor. This is because not all pointer conversions are pointer-interconvertible. That is, some pointer conversions (such as multiple or virtual inheritance) produce a pointer with different bits. Calling the original deleter, which expects a T* and not a U*, with the converted U*, would then fail. The current approach is only safe in two cases:

• Where std::is_pointer_interconvertible_base_class<T,U> is true and the destructor of T is either virtual or trivial, or
• Where std::is_layout_compatible<T,U>, std::is_trivially_destructible<T> and std::is_trivially_destructible<U> are all true. (The standard does not say std::unique_ptr should support this case.)

At present, however, the implementation supports this for all possible conversions, including arbitrary one-way custom conversion operators. The only way to make this work robustly would be to store the closure of the deleter and the original pointer inside each instance.

If you aren’t going to go through all that, you could declare non-pointer-interconvertible aliasing unsupported, add a check for that to your restrict clauses, and keep the simple, high-performance implementation you have now. In that case, you want to declare your deleter with the [[no_unique_address]] attribute, so that the vast majority of deleters, which are unit types whose instance contain nothing, don’t need to be created and stored as objects.

Otherwise, you could put a/*** BIG IMPORTANT WARNING ***/ in your header file that people are only supposed to pass in a deleter that still works, somehow, with every possible conversion to a base pointer. And then say, the bugs this causes are on them for not reading the documentation.

Finally, since you ask, the converting constructor appears to be number 6 on Cppreference, as of July 2023.