Move semantics and function order evaluation

Question

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

Batch

If you can change B's constructor you don't need to do any of this. Just have a single argument, unique_ptr<A> and do the copy of a->x in the constructor's initialization list. – Keitloa

Well, I didn't want to change B's interface in such a way to support this specific usage. Initializing x with a->x might not be a expected thing to do and thus should not require a special case from B. It depends on the context, but it might be more natural for a constructor taking only the unique_ptr to initialize x to some default constant instead of a->x. If we change B to take the unique_ptr by rvalue reference, we give callers more flexibility for free and don't change the interface. I don't see any reason why the unique_ptr argument should be passed by value here. – Batch

You're right, there's no downside to passing by rvalue reference here. Another possibility is keeping the existing B constructor, and adding an overload that takes only a unique_ptr<A>. In that case it's implied B will initialize x from a->x. Which one you choose really depends on the intended use of your class. – Keitloa

See Scott Meyers' post that this question inspired: scottmeyers.blogspot.com.au/2014/07/… – Bardo

Answer 1

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.

Keitloa

Did not know that rule. Does that come from C? That is, does struct initialization in C maintain this quality? – Tautologism

@RyanHaining In case of POD structs, the initialization using braces is aggregate initialization, which has always been part of C++ (inherited from C). C++11 added list initialization (which is what I'm using here), which also encompasses aggregate initialization. I'm pretty sure aggregate initialization has always has a specified order of evaluation from left to right because struct, or class, members are always initialized in the order they're defined in. – Keitloa

I'll look at designated initializers in C99, it would make sense that they'd still be evaluated in the order that they're defined, but if the standard specifies "order they appear" I could be wrong. – Tautologism

@RyanHaining Good point about designated initializers, I don't know what C99 states about those. Anyway, everything I said only applies to C++. It's possible it's also true for C, but I don't know that for sure. – Keitloa

NOTE: gcc has, for a long time, been violating the evaluation order inside braced-init-list's; the bug was fixed 2014-07-01 (trunk). – Orfurd

Answer 2

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
    {}
};

Tybie

Why does the first move not nullify a? Is it because std::move is basically just a cast? – Ssm

@ChrisDrew Yes, all you're doing there is binding it to a reference. The actual moving of the internals will be done in C's private constructor's initialization list when the argument for B's constructor is constructed. – Keitloa

Answer 3

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};

Batch

@Deduplicator: No one's passing a.get() (the raw pointer the unique_ptr contains) – Conjunctiva

@Jarod42: This is the asker's own answer. – Mercie

If we are allowed to change B, I would suggest the (pragmatic) solution of adding a constructor to B that only takes a unique_ptr<A> which sets x internally from a. – Ssm

Answer 4

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.

Illustrious

Think again. Even though the move(a) by itself does not move anything, the initialization of the B's second parameter does move from a. If this happens before a->x is evaluated, then there is undefined behavior. – Adviser