CODEWITHSUNDEEP
c++c++11initializationmove-semanticsunspecified-behavior
Matthew Fioravante
Matthew Fioravante

Reputation: 1498

Move semantics and function order evaluation

Suppose I have the following:

#include <memory>
struct A { int x; };

class B {
  B(int x, std::unique_ptr<A> a);
};

class C : public B {
  C(std::unique_ptr<A> a) : B(a->x, std::move(a)) {}
};

If I understand the C++ rules about "unspecified order of function parameters" correctly, this code is unsafe. If the second argument to B's constructor is constructed first using the move constructor, then a now contains a nullptr and the expression a->x will trigger undefined behavior (likely segfault). If the first argument is constructed first, then everything will work as intended.

If this were a normal function call, we could just create a temporary:

auto x = a->x
B b{x, std::move(a)};

But in the class initialization list we don't have the freedom to create temporary variables.

Suppose I cannot change B, is there any possible way to accomplish the above? Namely dereferencing and moving a unique_ptr in the same function call expression without creating a temporary?

What if you could change B's constructor but not add new methods such as setX(int)? Would that help?

Thank you

Upvotes: 52

Views: 3845

Answers (4)

John Alexander
John Alexander

Reputation: 1

The code contains no undefined behaviour. This is a common mis-conception that std::move() actually performs a move, it does not. std::move() simply casts the input to an r-value reference which is a semantic compile time change and has no runtime code. Therefore in the statement:

B(a->x, std::move(a))

The state of 'a' is not modified by the std::move() call therefore there is no undefined behaviour regardless of the evaluation ordering.

Upvotes: 0

Matthew Fioravante
Matthew Fioravante

Reputation: 1498

Praetorian's suggestion of using list initialization seems to work, but it has a few problems:

  1. If the unique_ptr argument comes first, we're out of luck
  2. Its way too easy for clients of B to accidentally forget to use {} instead of (). The designers of B's interface has imposed this potential bug on us.

If we could change B, then perhaps one better solution for constructors is to always pass unique_ptr by rvalue reference instead of by value.

struct A { int x; };

class B {
  B(std::unique_ptr<A>&& a, int x) : _x(x), _a(std::move(a)) {}
};

Now we can safely use std::move().

B b(std::move(a), a->x);
B b{std::move(a), a->x};

Upvotes: 11

Jarod42
Jarod42

Reputation: 217255

As alternative to Praetorian's answer, you can use constructor delegate:

class C : public B {
public:
    C(std::unique_ptr<A> a) :
        C(a->x, std::move(a)) // this move doesn't nullify a.
    {}

private:
    C(int x, std::unique_ptr<A>&& a) :
        B(x, std::move(a)) // this one does, but we already have copied x
    {}
};

Upvotes: 33

Praetorian
Praetorian

Reputation: 109119

Use list initialization to construct B. The elements are then guaranteed to be evaluated from left to right.

C(std::unique_ptr<A> a) : B{a->x, std::move(a)} {}
//                         ^                  ^ - braces

From §8.5.4/4 [dcl.init.list]

Within the initializer-list of a braced-init-list, the initializer-clauses, including any that result from pack expansions (14.5.3), are evaluated in the order in which they appear. That is, every value computation and side effect associated with a given initializer-clause is sequenced before every value computation and side effect associated with any initializer-clause that follows it in the comma-separated list of the initializer-list.

Upvotes: 48

Related Questions