1
\$\begingroup\$

For a couple of days, I have been trying to implement an own templated class called owner_ptr (for my little project) whose instances has ownership over a dynamically allocated object (or array). I want this template to support the deletion of dynamically allocated arrays too and also make it possible to pass ownership between owner_ptr objects. I've tried out many things and ideas (the last one was this) but each of them had some drawbacks.

The necessity of passing ownership between owner_ptr objects causes that you must modify the pointer of the passed owner_ptr object during copy-constructing or assignment (you have to set the pointer to NULL). Copy-constructor and assignment operator with const-reference is necessary to make this class work with std::vector.

template <typename T>
class owner_ptr
{
    /* Private members */

public:
    owner_ptr(const owner_ptr<T>&);
    owner_ptr& operator=(const owner_ptr<T>&);

   /* Other public member functions */
}

But how can I modify the objects if I have const-reference to them? The problem can be solved if I store the encapsulated pointer as a mutable member variable.

Here is my implementation:

template <typename T>
class owner_ptr
{
    mutable T* p;

    T* get_ownership() const
    {
        T* tmp = p;
        p = NULL;
        return tmp;
    }

public:
    explicit owner_ptr(T* ptr = NULL) : p(ptr) {}
    owner_ptr(const owner_ptr<T>& orig) : p(orig.get_ownership()) { }

    owner_ptr& operator=(const owner_ptr<T>& rhs)
    {
        if (this != &rhs)
        {
            this->pass(rhs.get_ownership());
        }
        return *this;
    }

    void pass(T* ptr = NULL)
    {
        if (p != ptr)
        {
            delete p;
            p = ptr;
        }
    }

    T* get() const { return p; }
    T* operator->() const { return p; }
    T& operator*() const { return *p; }

    ~owner_ptr()
    {
        this->pass();
    }
};

And a specialization for arrays (almost the same):

template <typename T>
class owner_ptr<T[]>
{
    mutable T* p;

    T* get_ownership() const
    {
        T* tmp = p;
        p = NULL;
        return tmp;
    }

public:
    explicit owner_ptr(T* ptr = NULL) : p(ptr) {}
    owner_ptr(const owner_ptr<T[]>& orig) : p(orig.get_ownership()) { }

    owner_ptr& operator=(const owner_ptr<T[]>& rhs)
    {
        if (this != &rhs)
        {
            this->pass(rhs.get_ownership());
        }
        return *this;
    }

    void pass(T* ptr = NULL)
    {
        if (p != ptr)
        {
            delete[] p;
            p = ptr;
        }
    }

    T* get() const { return p; }
    T* operator->() const { return p; }
    T& operator*() const { return *p; }
    T& operator[](int i) const { return p[i]; }

    ~owner_ptr()
    {
        this->pass();
    }
};

I tested the code and it seems to be working. std::vector also accepts this template, so I can use it for creating heterogeneous containers.

My questions:

  • Is it a bad idea to use mutable to solve the ownership-passing problem? Are there any scenarios where it can cause undefined behaviour?
  • Is there anything that I implemented incorrectly in the template?
  • Any affect on performance?
  • Before C++11, why did we have only the silly std::auto_ptr which couldn't be used with std::vector?

NOTE: I'm doing this for learning purposes. This question is totally independent from the features and smart-pointers of C++11.

\$\endgroup\$
  • 1
    \$\begingroup\$ Sounds like you are trying to reimplement std::unique_ptr without move semantics. That's impossible. std::auto_ptr can't be used with std::vector, because the copy c'tor passes ownership. std::unique_ptr fixes this by not being copyable, but only movable. Now, you can emulate move semantics in C++03, but third party code won't be able to use your emulation. So, you still can't use your class together with std::vector. \$\endgroup\$ – D Drmmr Aug 15 '17 at 0:14
  • 1
    \$\begingroup\$ To answer your final question - why, up to C++11, the only smart pointer was std::auto_ptr: because it's a hard problem, and required the invention of move semantics, which in turn caused the invention of rvalue references and substantial changes to the language to support this part of the Standard Library. It's not the kind of thing that can be quietly added as an afterthought. \$\endgroup\$ – Toby Speight Aug 15 '17 at 8:01
  • \$\begingroup\$ @DDrmmr - Can you explain it a bit clearer, why I can't still use it with std::vector? In fact, I was able to push_back owner_ptr objects into the container. Also, what is considered to be third party code? I'm using only the C++03 standard library. I've created this template for personal use only. \$\endgroup\$ – Gergely Tomcsányi Aug 15 '17 at 10:41
  • \$\begingroup\$ @TobySpeight - I just don't understand, why they couldn't add a better auto_ptr to the standard library before move semantics and rvalue reference were invented. If I was able to create a better one with C++03 tools... Maybe it's an ugly solution, but it would have been better than the almost useless auto_ptr before C++11. \$\endgroup\$ – Gergely Tomcsányi Aug 15 '17 at 10:49
  • \$\begingroup\$ You can use auto_ptr in vector but it has severe limitations that are really easy to violate. The real problem is that violating these constraints did not cause a compiler error and resulted in some very strange and unexpected consequence. \$\endgroup\$ – Martin York Aug 15 '17 at 22:32
3
\$\begingroup\$

OK. Lets ignore that you are screwing around with mutable.

No Strong Exception Guarantee.

void pass(T* ptr = NULL)
{
    if (p != ptr)
    {
        delete[] p;   // if p throws then your object is left in
                      // an invalid state and you have leaked the
                      // pointer `ptr`
        p = ptr;
    }
}

You should do this in an exception safe way. Which requires the use of a temporary.

void pass(T* ptr = NULL)
{
    if (p != ptr)
    {
        T* tmp = p;
        p = ptr;
        delete[] tmp;   // now it is safe to delete.
                        // even if it throws this object is consistent
                        // and you have not leaked ptr.
    }
}

Double ownership check

Both this method and the previous check for self assignment. Because this function calls the above function you perform a self assignment check twice when you use the assignment operator.

owner_ptr& operator=(const owner_ptr<T>& rhs)
{
    if (this != &rhs)  // Check here
    {
        this->pass(rhs.get_ownership());  // Check inside pass()
    }
    return *this;
}

I would change both of these methods to call a private method that does the check and work.

owner_ptr& operator=(const owner_ptr<T[]>& rhs)
{
    private_pass(rhs.get_ownership());
    return *this;
}

void pass(T* ptr = NULL)
{
    private_pass(ptr);
}
private:
void private_pass(T* ptr)
{
    if (p != ptr)
    {
        T* tmp = p;
        p = ptr;
        delete[] tmp;
    }
}

No Throwing methods

Since the get_ownership() method is guaranteed not to throw you should probably mark it as such.

T* get_ownership() throw();

Questions:

Is it a bad idea to use mutable to solve the ownership-passing problem?

Probably. Mutable is really for designating members that are not part of the objects actual state (ie temporary caches).

Are there any scenarios where it can cause undefined behaviour?

Don't think so. You will just loose out on certain optimizations.

Is there anything that I implemented incorrectly in the template?

Don't see anything.

Any affect on performance?

Yes. As noted above the compiler can not apply certain optimizations when a member is marked mutable.

Before C++11, why did we have only the silly std::auto_ptr which couldn't be used with std::vector?

Because the language did not have the features required to implement modern smart pointers. The auto_ptr was the first attempt at a smart pointer and it worked to a certain extent it just had limitations.

It took them 8 years to improve the language enough so that smart pointers could be implemented properly.

\$\endgroup\$
  • \$\begingroup\$ @Incomputable: Fixed. This is why compilers are good. \$\endgroup\$ – Martin York Aug 16 '17 at 0:06
  • \$\begingroup\$ or a fellow reviewer :) by the way, I finally got AST builder working, took me long enough. \$\endgroup\$ – Incomputable Aug 16 '17 at 0:08
  • \$\begingroup\$ Thanks for the clear and useful answer! I will correct my mistakes. \$\endgroup\$ – Gergely Tomcsányi Aug 16 '17 at 8:54
  • \$\begingroup\$ @GergelyTomcsányi Your attempted edit completed changed the meaning. Please don't edit my code. I am fine if you see grammatical or spelling errors in the text fix those. But NEVER touch the code. I will fix it if you tell me in comments that there is an issue. \$\endgroup\$ – Martin York Aug 29 '17 at 20:31
  • \$\begingroup\$ @LokiAstari - Sorry, I thought I can sign these errors through edit too. If you look at operator=() method, it doesn't check self-assignment. So, if you call this function by owner_ptr<int[]> holder(new int[5]); holder = holder;, then in operator=(), rhs.get_ownership() is called which sets the pointer of rhs (= holder) to NULL, then, in private_pass(), you set the pointer back to the original. You lose performance, if it is not optimized somehow. That's why I thought it's a better idea to do the checks in operator=() and pass() and take out the check from private_pass(). \$\endgroup\$ – Gergely Tomcsányi Aug 29 '17 at 21:24

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.