Is it a good habit to reset self to nullptr when use move-constructor?

Question

in C++11, the move-constructor/operator support resource/memory move.

This is my example:

class A {
public:
    A() : table_(nullptr), alloc_(0) {}
    ~A()
    {
        if (table_)
            delete[] table_;
    }

    A(const A & other)
    {
        // table_ is not initialized
        // if (table_)
        //    delete[] table_;
        table_ = new int[other.alloc_];
        memcpy(table_, other.table_, other.alloc_ * sizeof(int));
        alloc_ = other.alloc_;
    }
    A& operator=(const A & other)
    {
        if (table_)
            delete[] table_;
        table_ = new int[other.alloc_];
        memcpy(table_, other.table_, other.alloc_ * sizeof(int));
        alloc_ = other.alloc_;
        return *this;
    }

    A(A && other)
    {
        // table_ is not initialized in constructor
        // if (table_)
        //    delete[] table_;
        table_ = other.table_;
        alloc_ = other.alloc_;
    }

    A& operator=(A && other)
    {
        if (table_)
            delete[] table_;
        table_ = other.table_;
        alloc_ = other.alloc_;
    }

private:
    int *table_;
    int alloc_;
};

It seems good, but sometimes I want to move a local variable, like this:

class B {
private:
    A a_;

public:
    void hello()
    {
        A tmp;
        // do something to tmp
        a_ = std::move(tmp);
        // tmp.~A() is called, so a_ is invalid now.
    }
};

when the function end, tmp.~A() will be called, at this time, a_ and tmp has the same table_ pointer, when tmp delete[] table_, a_'s table_ will be invalid.

I'm wandering when should I use std::move to assign tmp to a_, without copy.

with help of the answers, I modify the A's move-constructor like this:

class A {
private:
    void reset()
    {
        table_ = nullptr;
        alloc_ = 0;
    }

public:

    A(A && other)
    {
        table_ = other.table_;
        alloc_ = other.alloc_;
        other.reset();
    }

    A& operator=(A && other)
    {
        std::swap(table_, other.table_);
        std::swap(alloc_, other.alloc_);
    }
};

In this code, when I move something, I will swap new and old reference, so the old tmp will delete[] original a_ table_, which is useless.

this is a good habit to do this.

Answer 1

When you move from other in A(A && other), you should also set to nulltpr its moved data members. So the fixed code should looks as follows:

A(A && other)
{
    //if (table_)
    //    delete[] table_; // no need for this in move c-tor
    table_ = other.table_;
    other.table_ = nullptr;
    alloc_ = other.alloc_;
    other.alloc_ = nullptr;
}

A& operator=(A && other)
{
    // as n.m. has pointed out, this move assignment does not 
    // protect against self assignment. One solution is to use
    // swap aproach here. The other is to simply check if table_ == other.table_. 
    // Also see here for drawbacks of swap method:
    // http://scottmeyers.blogspot.com/2014/06/the-drawbacks-of-implementing-move.html
    delete[] table_;
    table_ = other.table_;
    other.table_ = nullptr;
    alloc_ = other.alloc_;
    other.alloc_ = nullptr;
    return *this;
}

This puts other in what standard calls valid but unspecified state.

you may also use std::swap as follows:

A(A && other)
{
    table_ = other.table_;
    alloc_ = other.alloc_;
}

A& operator=(A && other)
{
    std::swap(table_, other.table_);
    std::swap(alloc_, other.alloc_);
    return *this;
}

this way deallocation will be done when the moved from object is being destroyed.