8
\$\begingroup\$

Wrote a couple of blog articles about smart pointers.

So I suppose it time to get the result reviewed.
This is not supposed to be a replacement for the standard smart pointers. It is more a simple place to start for people trying to learn some of the basics. I am trying to show of the basic things you have to do in order to get one off the ground as a learning processes.

I suppose one of the main things missing is handling arrays.

#include <cstddef>
#include <utility>

namespace Loki
{
template<typename T>
class UniquePtr
{
    T*      data;
    public:
        // Default constructor.
        UniquePtr()
            : data(nullptr)
        {}
        // Take ownership of a pointer
        // Note: Do this explicitly to avoid accidental conversion
        //       If we allowed accidental conversion while passing to a function.
        //       The pointer would be deleted on function return so an unsuspecting
        //       user would then be using a deleted pointer. So we must make
        //       taking ownership be explicit.
        explicit UniquePtr(T* data)
            : data(data)
        {}
        // Disable copying.
        UniquePtr(UniquePtr const&)             = delete;
        UniquePtr& operator=(UniquePtr const&)  = delete;
        // Allow move
        UniquePtr(UniquePtr&& move)
            : data(nullptr)
        {
            // Note: data must be null before the swap
            std::swap(data, move.data);
        }
        UniquePtr& operator=(UniquePtr&& move)
        {
            move.swap(*this);
            return *this;
        }
        // Allow the use of `nullptr directly
        // Because of the explicit above you can not pass a nullptr to functions
        // that take a UniquePtr as an argument you have to explicitly create the
        // UniquePtr to pass. `nullptr` is a unique case and it is safe to
        // dynamically create the UniquePtr on the fly. This constructor allows this.
        UniquePtr(std::nullptr_t)
            : data(nullptr)
        {}

        // Conversion Constructors.
        //      Note: These will only compile if U is derived from T
        template<typename U>
        UniquePtr(U* data)
            : data(data)
        {}
        template<typename U>
        UniquePtr(UniquePtr<U>&& move)
            : data(nullptr)
        {
            move.swap(*this);
        }
        template<typename U>
        UniquePtr operator=(UniquePtr<U>&& move)
        {
            // Note:
            //  We can not swap *this and move.
            //  Because what we have stored locally may not be a U
            //  so it can not be moved to a U. So we must store that
            //  locally in old (this making this->data = nullptr)
            UniquePtr<T>    old(std::move(*this));

            // Now that this->data is a nullptr we can swap them.
            move.swap(*this);
            return *this;
        }

        // And of course delete when it goes out of scope.
        ~UniquePtr()
        {
            delete [] data;
        }

        void swap(UniquePtr& other)  noexcept
        {
            using std::swap;
            swap(data, other.data);
        }
        // Release the data from ownership.
        T* release()
        {
            T* result = nullptr;
            std::swap(result,data);
            return result;
        }

        // Common operations performed on a uniquePtr
        // Note the state of `data` does not affect the state of the
        // smart pointer.
        T& operator&()  const       {return *data;}
        T* operator->() const       {return  data;}

        // Common tests
        bool isEmpty() const        {return  data;}
        operator bool()const        {return  data;}

        // Just get raw pointer.
        T*   get()     const        {return  data;} // Note the method is const
                                                    // even though we don't return a const pointer.
                                                    // This is deliberate. We are allowing raw access
                                                    // to the data that does not involve the objects
                                                    // ownership.
};
}

int main()
{
    Loki::UniquePtr<int>      data(new int(5));
}

// Done
\$\endgroup\$
  • 1
    \$\begingroup\$ Typo: T& operator&() should be T& operator*(). \$\endgroup\$ – Apples Jun 11 '15 at 19:16
  • \$\begingroup\$ Nobody caught it. But move operators should be declared noexcept. They still work if they are not declared this way. But the standard library get to perform a whole bunch more optimizations of you make sure the move constructor/assignemnt operator are noexcept. \$\endgroup\$ – Martin York Jun 12 '15 at 20:55
  • \$\begingroup\$ Actually, my understanding is that those optimizations will still happen. I believe the standard library will use this pseudo logic: if (noexcept(move)) { move; } else if (copy) { copy; } else { move; }. Since you provide no copy operations, you're forcing a move, but losing strong exception safety (but not really, because your move can't throw). Although you might still mark it noexcept for documentation and symmetry with other classes that are copyable. Quick edit: I changed my mind; using noexcept can do no harm, so you should probably use it just to future-proof the class. \$\endgroup\$ – Apples Jun 12 '15 at 23:57
  • \$\begingroup\$ @AlchemicalApples: There are certain operations that can't use move unless they are noexcept and still maintain certain guarantees. Example when array re-sizes and you start coping elements from the old buffer to the new buffer. If there is an exception you can stop the copy revert the array back to its original state before rethrowing the exception. If you are doing a move this is not possible (because you no longer have the original state because you moved half of it). Therefore if the move is not marked as noexcept a resize is forced to use copy on each of the elements. \$\endgroup\$ – Martin York Jun 14 '15 at 2:07
  • \$\begingroup\$ But if you guarantee that move is non throwing then you can safely move the content of the old buffer to the new buffer and thus the standard libraries will use the move during an vector resize rather than a copy. \$\endgroup\$ – Martin York Jun 14 '15 at 2:08
3
\$\begingroup\$

Explicit operator bool()

It would be best to mark operator bool() as explicit.

explicit operator bool() const { return data; }

Otherwise, this code will be allowed:

Loki::UniquePtr<int>   iptr (new int  (5  ));
Loki::UniquePtr<float> fptr (new float(7.2));
assert(iptr == fptr);

Because iptr and fptr will be converted to bool and compared as true == true.

Marking the conversion explicit will still allow code such as if (iptr) {...} and (iptr && *iptr == 5).

\$\endgroup\$
3
\$\begingroup\$

Contrived bool conversion:

@AlchemicalApples beat me to it, but I'm going to say it anyways.

Your implementation looks quite good, however, there is the issue of operator bool and the implicit conversions it allows.

I've dug up my copy of Modern C++ Design and re-read the chapter on smart pointers. It goes into a lot of detail about the issues related to providing safe and as idiomatic as possible comparison operators. I suggest that you read it yourself.

The problem with overloading operator bool is that code such at the following will compile:

int main()
{
    Loki::UniquePtr<int>   data1(new int(5));
    Loki::UniquePtr<float> data2(new float(6.0));

    if (data1 == data2) // Will be true!
    {
        std::cout << "Problem!\n";
    }

    bool b = data1; // Unusual, but...

    if ((data1 * 5) == 200) // Starts to behave like an integer, doesn't it?
    {
    }
}

It is still useful, however, to be able to test a pointer in the traditional if (ptr) ... way. To achieve that syntax, the best way is to explicitly provide each comparison operator (==, !=, etc) and then use the "tester type" trick presented in the book, which boils down to replacing the bool operator with a conversion to a dummy pointer type that can't be deleted:

class UniquePtr
{
    // Don't let it get deleted outside.
    class Tester 
    {
        void operator delete (void *);
    };

public:

    // Allows safe and correct "if (ptr)..." checks and is an actual pointer type.
    operator Tester*() const
    {
        if (! data)
        {
            return nullptr;
        }
        static Tester tester;
        return &tester;
    }
};

The example above is adapted directly from the book, so again, I recommend reading it to get a much more complete explanation of why this works.

new mixed with delete[]:

Another thing out of place that I've spotted is that your destructor is deleting an array, while in your example, it allocated a single element:

~UniquePtr()
{
    delete [] data;
}

This might work quietly when dealing with native types that have no destructors, but will fail catastrophically for types that have destructors. Nevertheless, it is a mistake and should be fixed.

\$\endgroup\$
  • 1
    \$\begingroup\$ +1 for pointing out new mixed with delete[], that's actually a huge issue even for simple types, depending on how the compiler handles allocations. \$\endgroup\$ – Apples Jun 11 '15 at 19:01
  • \$\begingroup\$ Why use the Tester object when @AlchemicalApples explicit seems to achieve the same result (only allowing conversion in a boolean context). \$\endgroup\$ – Martin York Jun 11 '15 at 20:53
  • \$\begingroup\$ I loved Alexandrescu book when it first came out in 2001. But I believe that advice is dated and there are better techniques now. Though you are correct my version is broken. \$\endgroup\$ – Martin York Jun 11 '15 at 20:56
  • 1
    \$\begingroup\$ Found it. Bit of a search. The explicit operator was introduced to remove the need for this type of hackary. open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2435.htm If anybody has a more recent paper that would be good. \$\endgroup\$ – Martin York Jun 11 '15 at 21:20
  • 1
    \$\begingroup\$ @LokiAstari @glampert Sorry I'm late, I could have shed some light. The explicit keyword was introduced in C++11 for conversion operators to specifically address this problem. It can be applied to conversion operators (explicit operator bool()) and also conversion constructors (explicit UniquePtr(T* data)). Readable documentation can be found here. It is specified in sections 7.1.2, 12.3.1, and 12.3.2 of ISO N3337. \$\endgroup\$ – Apples Jun 12 '15 at 4:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.