My implementation for std::unique_ptr I just finished learning about move semantics and realized that a nice practical example for this concept is unique_ptr (it cannot be copied, only moved).

For learning purposes, and as a personal experiment, I proceed to try to create my implementation for a smart unique pointer:

template&lt;typename T&gt;
class unique_ptr {
private:
    T* _ptr;
public:
    unique_ptr(T&amp; t) {
       _ptr = &amp;t;
    }
    unique_ptr(unique_ptr&lt;T&gt;&amp;&amp; uptr) {
       _ptr = std::move(uptr._ptr);
       uptr._ptr = nullptr;
    }
    ~unique_ptr() {
       delete _ptr;
    }
    unique_ptr&lt;T&gt;&amp; operator=(unique_ptr&lt;T&gt;&amp;&amp; uptr) {
       if (this == uptr) return *this;
       _ptr = std::move(uptr._ptr);
       uptr._ptr = nullptr;
       return *this;
    }

    unique_ptr(const unique_ptr&lt;T&gt;&amp; uptr) = delete;
    unique_ptr&lt;T&gt;&amp; operator=(const unique_ptr&lt;T&gt;&amp; uptr) = delete;
};


For a small set of test cases, this is working like the real unique_ptr.

However, it just seems too simple enough.

I have two questions regarding this code:


Is it well-formed? i.e. does it follow common C++ standard and patterns (for example, should private members be declared before public ones?
Am I missing something regarding functionality? Is there maybe a bug in my code that I'm not seeing?
Overall

Ohhhhh.


  Is it well-formed? 


It compiles, so yes.


  i.e. does it follow common C++ standard and patterns (for example, should private members be declared before public ones?


Personally I think so.

* Private Variables
* Public
    * Constuctor / Destructor
    * Copy Semantics
    * Move Semantics
    * Swap
    * Other Public Interface
    * Friends
* Private
    * Methods as appropriate


When reading the code I want to know the members so I can verify that the constructors initialize them all, as a result I usually put them first. But other people prefer to put all private stuff at the bottom.


  Am I missing something regarding functionality?


Yes. Quite a lot.


  Is there maybe a bug in my code that I'm not seeing?


Yes. It potentially leaks on assignment.

Code Review

Constructing from object!

    unique_ptr(T&amp; t) {
       _ptr = &amp;t;
    }


That's exceedingly dangerous:

{
    int x;
    unique_ptr&lt;int&gt;  data(x);
}
// Here the unique ptr calls delete on an automatic variable.


Use member initializing list.

You should always attempt to use the member initializer list for initializing members. Any non-trivial object will have its constructor called before the initializer code is called and thus it is inefficient to then re-initialize it in the code.

    unique_ptr(unique_ptr&lt;T&gt;&amp;&amp; uptr)
        : _ptr(std::move(uptr.ptr))
    {
       uptr._ptr = nullptr;
    }


Member variable Names

Prefer not to use _ as the first character in an identifier name.

T* _ptr;  // Technically OK.


Even if you know all the rules of when to use them most people don't so they are best avoided. If you must have a prefix to identify members use m_ - but if you name your member variables well then there is no need for any prefix (in my opinion prefixes makes the code worse not better, because you are relying on unwritten rules. If you have good well-defined names (see self-documenting code) then members should be obvious).

NoExcept

The move operators should be marked as noexcept. 

When used with standard containers this will enable certain optimizations. This is because if the move is noexcept then certain operations can be guaranteed to work and thus provide the strong exception guarantee.

    unique_ptr(unique_ptr&lt;T&gt;&amp;&amp; uptr) noexcept
                              //     ^^^^^^^^

    unique_ptr&lt;T&gt;&amp; operator=(unique_ptr&lt;T&gt;&amp;&amp; uptr) noexcept
                                            //     ^^^^^^^^


Leak in assignment

Note: Your current assignment potentially leaks. If this currently has a pointer assigned then you overwrite it without freeing.

    unique_ptr&lt;T&gt;&amp; operator=(unique_ptr&lt;T&gt;&amp;&amp; uptr) {
       if (this == uptr) return *this;

       // Here you overwrite _ptr
       // But if it has a value then you have just leaked it.
       _ptr = std::move(uptr._ptr);

       uptr._ptr = nullptr;
       return *this;
    }


Checking for this pessimization

    unique_ptr&lt;T&gt;&amp; operator=(unique_ptr&lt;T&gt;&amp;&amp; uptr) {
       if (this == uptr) return *this;
       _ptr = std::move(uptr._ptr);
       uptr._ptr = nullptr;
       return *this;
    }


Yes you do need to make it work when there is self assignment. But in real code the self assignment happens so infrequently that this test becomes a pessimization on the normal case (same applies for copy operation). There have been studies on this (please somebody post a link; I have lost mine and would like to add it back to my notes).

The standard way of implementing move is via swap. Just like Copy is normally implemented by Copy and Swap.

    unique_ptr(unique_ptr&lt;T&gt;&amp;&amp; uptr) noexcept
        : _ptr(nullptr)
    {
        this-&gt;swap(uptr);
    }
    unique_ptr&lt;T&gt;&amp; operator=(unique_ptr&lt;T&gt;&amp;&amp; uptr) noexcept
    {
        this-&gt;swap(uptr);
        return *this;
    }
    void swap(unique_ptr&lt;T&gt;&amp; other) noexcept
    {
        using std::swap;
        swap(_ptr, other._ptr);
    }


Using the swap technique also delays the calling of the destructor on the pointer for the current object. Which means that it can potentially be re-used. But if it is going out of scope the unique_ptr destructor will correctly destroy it.

Summary

Good first try but still lots of issues.

Please read the article I wrote on unique_ptr and shared_ptr for lots more things you should implement.

Smart-Pointer - Unique Pointer
Smart-Pointer - Shared Pointer
Smart-Pointer - Constructors  

Some things you missed:


Constructor with nullptr
Constructor from derived type
Casting to bool
Checking for empty
Guaranteeing delete on construction failure.
Implicit construction issues
Dereferencing


When you have read all three articles then the bare bones unique_ptr looks like this:

namespace ThorsAnvil
{
    template&lt;typename T&gt;
    class UP
    {
        T*   data;
        public:
            UP()
                : data(nullptr)
            {}
            // Explicit constructor
            explicit UP(T* data)
                : data(data)
            {}
            ~UP()
            {
                delete data;
            }

            // Constructor/Assignment that binds to nullptr
            // This makes usage with nullptr cleaner
            UP(std::nullptr_t)
                : data(nullptr)
            {}
            UP&amp; operator=(std::nullptr_t)
            {
                reset();
                return *this;
            }

            // Constructor/Assignment that allows move semantics
            UP(UP&amp;&amp; moving) noexcept
            {
                moving.swap(*this);
            }
            UP&amp; operator=(UP&amp;&amp; moving) noexcept
            {
                moving.swap(*this);
                return *this;
            }

            // Constructor/Assignment for use with types derived from T
            template&lt;typename U&gt;
            UP(UP&lt;U&gt;&amp;&amp; moving)
            {
                UP&lt;T&gt;   tmp(moving.release());
                tmp.swap(*this);
            }
            template&lt;typename U&gt;
            UP&amp; operator=(UP&lt;U&gt;&amp;&amp; moving)
            {
                UP&lt;T&gt;    tmp(moving.release());
                tmp.swap(*this);
                return *this;
            }

            // Remove compiler generated copy semantics.
            UP(UP const&amp;)            = delete;
            UP&amp; operator=(UP const&amp;) = delete;

            // Const correct access owned object
            T* operator-&gt;() const {return data;}
            T&amp; operator*()  const {return *data;}

            // Access to smart pointer state
            T* get()                 const {return data;}
            explicit operator bool() const {return data;}

            // Modify object state
            T* release() noexcept
            {
                T* result = nullptr;
                std::swap(result, data);
                return result;
            }
            void swap(UP&amp; src) noexcept
            {
                std::swap(data, src.data);
            }
            void reset()
            {
                T* tmp = release();
                delete tmp;
            }
    };
    template&lt;typename T&gt;
    void swap(UP&lt;T&gt;&amp; lhs, UP&lt;T&gt;&amp; rhs)
    {
        lhs.swap(rhs);
    }
}


Test to make sure it compiles:

struct X {}; 
struct Y: public X {}; 

int main()
{
    ThorsAnvil::UP&lt;X&gt;   x(new X); 
    ThorsAnvil::UP&lt;Y&gt;   y(new Y); 

    x   = std::move(y);                  // This should be valid
    ThorsAnvil::UP&lt;X&gt;   z(std::move(y)); // This should be valid

    // In both these cases x represents an X* which is a base pointer
    // that should be able to point at objects of type Y* as these
    // are derived from X
}