52

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

Michael Dorst
  • 8,210
  • 11
  • 44
  • 71
Matthew Fioravante
  • 1,478
  • 15
  • 19

4 Answers4

47

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.

Praetorian
  • 106,671
  • 19
  • 240
  • 328
  • Did not know that rule. Does that come from C? That is, does struct initialization in C maintain this quality? – Ryan Haining Jul 18 '14 at 01:22
  • 1
    @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. – Praetorian Jul 18 '14 at 01:33
  • 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. – Ryan Haining Jul 18 '14 at 01:58
  • @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. – Praetorian Jul 18 '14 at 02:06
  • 9
    **NOTE**: `gcc` has, for a long time, been [violating the evaluation order](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51253) inside *braced-init-list's*; the bug was fixed **2014-07-01** (trunk). – Filip Roséen - refp Jul 18 '14 at 03:02
33

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
    {}
};
Jarod42
  • 203,559
  • 14
  • 181
  • 302
  • 1
    Why does the first move not nullify a? Is it because std::move is basically just a cast? – Chris Drew Jul 18 '14 at 05:30
  • 1
    @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. – Praetorian Jul 18 '14 at 06:44
11

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};
Matthew Fioravante
  • 1,478
  • 15
  • 19
0

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.

j6t
  • 9,150
  • 1
  • 15
  • 35
  • 1
    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. – j6t Apr 21 '23 at 11:27