C++ – Is This Move Constructor Code Safe?

c++c++11move-semantics

This is the move constructor of class X:

X::X(X&& rhs)
    : base1(std::move(rhs))
    , base2(std::move(rhs))
    , mbr1(std::move(rhs.mbr1))
    , mbr2(std::move(rhs.mbr2))
{ }

These are the things I'm wary about:

  1. We're moving from rhs twice and rhs isn't guaranteed to be in a valid state. Isn't that undefined behavior for the initialization of base2?
  2. We're moving the corresponding members of rhs to mbr1 and mbr2 but since rhs was already moved from (and again, it's not guaranteed to be in a valid state) why should this work?

This isn't my code. I found it on a site. Is this move constructor safe? And if so, how?

Best Answer

This is approximately how an implicit move constructor typically works: each base and member subobject is move-constructed from the corresponding subobject of rhs.

Assuming that base1 and base2 are bases of X that do not have constructors taking X / X& / X&& / const X&, it's safe as written. std::move(rhs) will implicitly convert to base1&& (respectively base2&&) when passed to the base class initializers.

EDIT: The assumption has actually bitten me a couple of times when I had a template constructor in a base class that exactly matched X&&. It would be safer (albeit incredibly pedantic) to perform the conversions explicitly:

X::X(X&& rhs)
    : base1(std::move(static_cast<base1&>(rhs)))
    , base2(std::move(static_cast<base2&>(rhs)))
    , mbr1(std::move(rhs.mbr1))
    , mbr2(std::move(rhs.mbr2))
{}

or even just:

X::X(X&& rhs)
    : base1(static_cast<base1&&>(rhs))
    , base2(static_cast<base2&&>(rhs))
    , mbr1(std::move(rhs.mbr1))
    , mbr2(std::move(rhs.mbr2))
{}

which I believe should exactly replicate what the compiler would generate implicitly for X(X&&) = default; if there are no other base classes or members than base1/base2/mbr1/mbr2.

EDIT AGAIN: C++11 ยง12.8/15 describes the exact structure of the implicit member-wise copy/move constructors.

Related Question