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

4
  • 1
    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.
    – Praetorian
    Jul 18, 2014 at 16:13
  • 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. Jul 18, 2014 at 16:26
  • 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.
    – Praetorian
    Jul 18, 2014 at 16:57
  • 4
    See Scott Meyers' post that this question inspired: scottmeyers.blogspot.com.au/2014/07/…
    – Jon
    Aug 5, 2014 at 1:08

4 Answers 4

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.

5
  • Did not know that rule. Does that come from C? That is, does struct initialization in C maintain this quality? Jul 18, 2014 at 1: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, 2014 at 1: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. Jul 18, 2014 at 1: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, 2014 at 2:06
  • 9
    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). Jul 18, 2014 at 3: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
    {}
};
2
  • 1
    Why does the first move not nullify a? Is it because std::move is basically just a cast?
    – Chris Drew
    Jul 18, 2014 at 5: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, 2014 at 6: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};
3
  • @Deduplicator: No one's passing a.get() (the raw pointer the unique_ptr contains)
    – Ben Voigt
    Jul 17, 2014 at 23:22
  • 1
    @Jarod42: This is the asker's own answer.
    – Peter O.
    Jul 18, 2014 at 1:20
  • 2
    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.
    – Chris Drew
    Jul 18, 2014 at 7:51
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.

1
  • 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 at 11:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.