(Why) should a move constructor or move assignment operator clear its argument?

Question

An example move constructor implementation from a C++ course I’m taking looks a bit like this:

/// Move constructor
Motorcycle::Motorcycle(Motorcycle&& ori)
    : m_wheels(std::move(ori.m_wheels)),
      m_speed(ori.m_speed),
      m_direction(ori.m_direction)
{
    ori.m_wheels = array<Wheel, 2>();
    ori.m_speed      = 0.0;
    ori.m_direction  = 0.0;
}

(m_wheels is a member of type std::array<Wheel, 2>, and Wheel only contains a double m_speed and a bool m_rotating. In the Motorcycle class, m_speed and m_direction are also doubles.)

I don’t quite understand why ori’s values need to be cleared.

If a Motorcycle had any pointer members we wanted to “steal”, then sure, we’d have to set ori.m_thingy = nullptr so as to not, for example, delete m_thingy twice. But does it matter when the fields contain the objects themselves?

I asked a friend about this, and they pointed me towards this page, which says:

Move constructors typically "steal" the resources held by the argument (e.g. pointers to dynamically-allocated objects, file descriptors, TCP sockets, I/O streams, running threads, etc), rather than make copies of them, and leave the argument in some valid but otherwise indeterminate state. For example, moving from a std::string or from a std::vector may result in the argument being left empty. However, this behaviour should not be relied upon.

Who defines what indeterminate state means? I don’t see how setting the speed to 0.0 is any more indeterminate than leaving it be. And like the final sentence in the quote says — code shouldn’t rely on the state of the moved-from Motorcycle anyway, so why bother cleaning it up?

Answer 1

They don't need to be cleaned. The designer of the class decided it would be a good idea to leave the moved-from object zero initialized.

Note that a situation where is does make sense is for objects managing resources that get released in the destructor. For instance, a pointer to dynamically allocated memory. Leaving the pointer in the moved-from object unmodified would mean two objects managing the same resource. Both their destructors would attempt to release.

Answer 2

Who defines what indeterminate state means?

The author of the class.

If you look at the documentation for std::unique_ptr, you'll see that after the following lines:

std::unique_ptr<int> pi = std::make_unique<int>(5);
auto pi2 = std::move(pi);

pi is actually in a very defined state. It will have been reset() and pi.get() will be equal to nullptr.

Answer 3

You're very probably correct that the author of this class is putting unnecessary operations in the constructor.

Even if m_wheels were a heap-allocated type, such as std::vector, there would still be no reason to "clear" it, since it is already being passed to its own move-constructor:

: m_wheels(std::move(ori.m_wheels)),   // invokes move-constructor of appropriate type

However, you have not shown enough of the class to permit us to know whether the explicit "clearing" operations are necessary in this particular case. As per Deduplicator's answer, the following functions are expected to behave correctly in the "moved-from" state:

// Destruction
~Motorcycle(); // This is the REALLY important one, of course
// Assignment
operator=(const Motorcycle&);
operator=(Motorcycle&&);

Therefore, you must look at the implementation of each of these functions in order to determine whether the move-constructor is correct.

If all three use the default implementation (which, from what you've shown, seems reasonable), then there's no reason to manually clear the moved-from objects. However, if any of these functions use the values of m_wheels, m_speed, or m_direction to determine how to behave, then the move-constructor may need to clear them in order to ensure the correct behavior.

Such a class design would be inelegant and error-prone, since typically we would not expect the values of primitives to matter in the "moved-from" state, unless the primitives are explicitly used as flags to indicate whether clean-up must occur in the destructor.

Ultimately, as an example for use in a C++ class, the move-constructor shown is not technically "wrong" in the sense of causing undesired behavior, but it seems like a poor example because it is likely to cause exactly the confusion that led you to post this question.

Answer 4

There are few things which must be safely doable with a moved-from object:

1. Destruct it.
2. Assign / Move to it.
3. Any other operations which explicitly say so.

So, you should move from them as fast as possible while giving those few guarantees, and more only if they are useful and you can fulfill them for free.
Which often means not clearing the source, and other times implies doing it.

As an addition, prefer explicitly defaulted over user-defined functions, and implicitly defined ones over both, for brevity and preserving triviality.

Answer 5

There is no standard behavior for this. Like with pointer you can still use them after you deleted them. Some people see you should not care and just not reuse the object but the compiler won't prohibit that.

Here is one blogpost (not by me) about this with some interesting discussion in the comment:

https://foonathan.github.io/blog/2016/07/23/move-safety.html

and the follow up:

https://foonathan.github.io/blog/2016/08/24/move-default-ctor.html

And here with also a recent Video chat about this topic with arguments discussing this:

https://www.youtube.com/watch?v=g5RnXRGar1w

Basically it's about treating a moved-from object like a deleted pointer or making it in a safe to be "moved-from-state"

Answer 6

Who defines what indeterminate state means?

The English language, I think. "Indeterminate" here has one of its usual English meanings, "Not determinate; not precisely fixed in extent; indefinite; uncertain. Not established. Not settled or decided". The state of a moved-from object is not constrained by the standard other than that it must be "valid" for that object type. It need not be the same every time an object is moved from.

If we were only talking about types provided by the implementation, then the proper language would be a "valid but otherwise unspecified state". But we reserve the word "unspecified" for talking about details of the C++ implementation, not details of what user code is allowed to do.

"Valid" is defined separately for each type. Taking integer types as an example, trap representations are not valid and anything that represents a value is valid.

That standard here is not saying that the move constructor must make the object be indeterminate, merely that it needn't put it into any determined state. So although you're correct that 0 is not "more indeterminate" than the old value, it is in any case moot since the move constructor needn't make the old object "as indeterminate as possible".

In this case, the author of the class has chosen to put the old object into one specific state. It's then entirely up to them whether they document what that state is, and if they do then it's entirely up to users of the code whether they rely on it.

I would usually recommend not relying on it, because under certain circumstances code that you write thinking of it semantically being a move, actually does a copy. For example, you put std::move on the right-hand side of an assignment not caring whether the object is const or not because it works either way, and then somebody else comes along and thinks "ah, it's been moved from, it must have been cleared to zeros". Nope. They've overlooked that Motorcycle is cleared when moved from, but const Motorcycle of course is not, no matter what the documentation might suggest to them :-)

If you're going to set a state at all, then it's really a coin toss which state. You could set it to a "clear" state, perhaps matching what the no-args constructor does (if there is one), on the basis that this represents the most neutral value there is. And I suppose on many architectures 0 is the (perhaps joint-)cheapest value to set something to. Alternatively you could set it to some eye-catcher value, in the hope that when someone writes a bug where they accidentally move from an object and then use its value, they'll think to themselves "What? The speed of light? In a residential street? Oh yeah, I remember, that's the value this class sets when moved-from, I probably did that".

Answer 7

The source object is an rvalue, potentially an xvalue. So the thing one needs to be concerned with is the imminent destruction of that object.

Resource handles or pointers are the most significant item that distinguishes a move from a copy: after the operation the ownership of that handle is assumed to be transferred.

So obviously, as you mention in the question, during a move we need to affect the source object so that it no-longer identifies itself as owner of transferred objects.

Thing::Thing(Thing&& rhs)
     : unqptr_(move(rhs.unqptr_))
     , rawptr_(rhs.rawptr_)
     , ownsraw_(rhs.ownsraw_)
{
    the.ownsraw_ = false;
}

Note that I don't clear rawptr_.

Here a design decision is made that, as long as the ownership flag is only true for one object, we don't care if there is a dangling pointer.

However, another engineer might decide that the pointer should be cleared so that instead of random ub, the following mistake results in a nullptr access:

void MyClass::f(Thing&& t) {
    t_ = move(t);
    // ...
    cout << t;
}

In the case of something as innocuous as the variables shown in the question, they may not need to be cleared but that depends on the class design.

Consider:

class Motorcycle
{
    float speed_ = 0.;
    static size_t s_inMotion = 0;
public:
    Motorcycle() = default;
    Motorcycle(Motorcycle&& rhs);
    Motorcycle& operator=(Motorcycle&& rhs);

    void setSpeed(float speed)
    {
        if (speed && !speed_)
            s_inMotion++;
        else if (!speed && speed_)
            s_inMotion--;
        speed_ = speed;
    }

    ~Motorcycle()
    {
        setSpeed(0);
    }
};

This is a fairly artificial demonstration that ownership isn't necessarily a simple pointer or bool, but can be an issue of internal consistency.

The move operator could use setSpeed to populate itself, or it could just do

Motorcycle::Motorcycle(Motorcycle&& rhs)
    : speed_(rhs.speed_)
{
    rhs.speed_ = 0;  // without this, s_inMotion gets confused
}

(Apologies for typos or autocorrects, typed on my phone)