4
\$\begingroup\$

I have Implemented Unique_Ptr with move semantics for the sake of Learning. Any suggestions, feed backs are highly appreciated.

// author @asad_nitp
#pragma once

namespace Smart_Pointers {
template<typename T>

class Unique_Ptr {
    T* m_ptr;
public:
    explicit Unique_Ptr(T* ptr = nullptr) noexcept
        : m_ptr(ptr) 
    {}

    ~Unique_Ptr() noexcept {
        if (m_ptr != nullptr) {
            delete m_ptr;
        }
        m_ptr = nullptr;
    }

    Unique_Ptr(const Unique_Ptr&) = delete;
    Unique_Ptr& operator = (const Unique_Ptr&) = delete;

    Unique_Ptr(Unique_Ptr&& ob) noexcept {
        m_ptr = ob.m_ptr;
        ob.m_ptr = nullptr;
    }
    Unique_Ptr& operator = (Unique_Ptr&& ob) noexcept {
        if (this != &ob) {
            m_ptr = ob.m_ptr;
            ob.m_ptr = nullptr;
        }
        return *this;
    }

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

    T& operator * () const {
        return *m_ptr;
    }

    T* get() const noexcept{
        return m_ptr;
    }

    T* release() noexcept {
        T* ans = m_ptr;
        m_ptr = nullptr;
        return ans;
    }

    void reset(T* ptr = nullptr) noexcept {
        T* old = m_ptr;
        m_ptr = ptr;
        if (old != nullptr)
            delete old;
    }

    void swap(Unique_Ptr& ob) noexcept {
        using std::swap;
        swap(m_ptr,ob.m_ptr);
    }

 };
} 
  //Examples
  int main() {
     Smart_Pointers::Unique_Ptr<int> up1(new int(5));
     Smart_Pointers::Unique_Ptr<int> up2(new int(6));
     up1 = up2; // Error
     up1 = std::move(up2); //fine

     return 0;
   }
\$\endgroup\$
  • 3
    \$\begingroup\$ At least one serios bug operator= will cause memory leak. Use swap. Use swap in the move constructor as well. delete shoudl be able to handle nullptr - no need for additional check \$\endgroup\$ – Artemy Vysotsky Sep 24 '17 at 12:52
5
\$\begingroup\$

As mentioned in the original comment you leak on assignment:

Unique_Ptr<int>   a = new int(1);
Unique_Ptr<int>   b;

a = b;  // You leak the original a pointer.

Looking at the assignment operator:

Unique_Ptr& operator = (Unique_Ptr&& ob) noexcept {
    if (this != &ob) {
        m_ptr = ob.m_ptr;     // Here you overwrite the current value.
                              // and thus leak its original value.
        ob.m_ptr = nullptr;
    }
    return *this;
}

As mentioned in the comments the standard way of implementing move semantics is:

Unique_Ptr(Unique_Ptr&& ob) noexcept
    : m_ptr(nullptr)
{
    swap(ob);
}
Unique_Ptr& operator = (Unique_Ptr&& ob) noexcept
{
    swap(ob);
    return *this;
}

This has a couple of benefits. Calling the destructor is not required. You are moving the actual data to the other object. Thus means the destructor of the other object will correctly handle destruction.

This also potentially allows for re-use. If the ob is in some other context and you the object could be re-used this can help in optimizations. If it can not be-reused it will correctly be deleted.

You remove the pesimizing check for self assignment.

You don't have a conversion too bool.

if (a) {
    // A is holding a value you can do stuff with it.
}

// Now you can do
if (a.get()) {
    // A is holding a value you can do stuff with it.
}
// But its not as neat. And the context changes.

This is simply fixed with:

// Note the explicit (added in C++11) to replace the safe-bool idiom
explicit operator bool() {
    return m_ptr;
}

Can you guarantee that the destructor of type T does not throw?

void reset(T* ptr = nullptr) noexcept;

I think not. I agree it should be but that is not something you can guarantee and thus your code should not guarantee that it does not. So this should probably be:

void reset(T* ptr = nullptr);

You can probably do some meta-programming to determine if the destructor is noexcept but I don't have that in my head.

Don't mark your destructor as noexcept.

~Unique_Ptr() noexcept; // this is not needed.
                        // Destructors are automatically noexcept

Your destructor is automatically noexcept unless specific conditions are met so the compiler will work it out correctly for you.

\$\endgroup\$

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.