I have an object Foo
owned by Owner
and no one else. I create an object of class Derived
that inherits Owner
, passing it a unique_ptr
to a newly created Foo
. Derived
has a member of class User
that also needs to use Foo
, but doesn't own it, so it gets only a raw pointer to it.
#include <memory>
#include <iostream>
using namespace std;
struct Foo {
void talk() { cout << "foo" << endl; }
};
struct User {
Foo *foo;
User(Foo *_foo): foo(_foo) {};
void use() { foo->talk(); } //potential disaster if foo's memory has changed
};
struct Owner {
unique_ptr<Foo> foo;
Owner(unique_ptr<Foo> _foo): foo(move(_foo)) {};
};
struct Derived : public Owner {
User user;
Derived(unique_ptr<Foo> _foo)
: Owner {move(_foo)},
user{_foo.get()} //potential disaster: _foo has already been move()'d, can't get()!
{};
void use() { user.use(); };
};
int main() {
Derived d(make_unique<Foo>());
//do other stuff
d.use(); //potential disaster
}
The problem is that in the initialization list of Derived
, I need to move()
the Foo
to pass it to Owner
's constructor (which takes a unique_ptr<Foo>
, because it is the owner), and also pass a raw pointer to the Foo
to User
. But by the time the User
gets initialized, the unique_ptr<Foo>
has already been moved and foo.get()
may point to an invalid location!
What's a good way to make sure User
gets a valid (non-owning) pointer to Foo
and Owner
still gets its unique_ptr<Foo>
?
You could delegate the work to a private constructor while duplicating the pointer:
struct Derived : public Owner {
User user;
Derived(unique_ptr<Foo> _foo)
: Derived(move(_foo), _foo.get())
{};
void use() { user.use(); };
private:
// Implement the real logic here
// As pointed by @Artyer in the comments, this r-value ref is required.
// Pass by value would again introduce UB in the calling code.
Derived(unique_ptr<Foo>&& _foo, Foo* ptr)
: Owner {move(_foo)},
user{ptr} {}
};
Note, that Derived(move(_foo),_foo.get())
is safe even though the order of evaluation is unspecified because std::move
is just a cast.
Just for clarity, this is not a potential disaster but a certain one
since the initializer list is evaluated from left to right.
Within the initializer-list of a braced-init-list, the initializer-clauses, including any that result from pack expansions ([temp.variadic]), 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. [Note: This evaluation ordering holds regardless of the semantics of the initialization; for example, it applies when the elements of the initializer-list are interpreted as arguments of a constructor call, even though ordinarily there are no sequencing constraints on the arguments of a call. — end note] .
@Artyer posted the correct explanation with a counter-example but since then has unfortunately deleted it.
There are two reasonable ways to write the delegating constructor:
Derived(unique_ptr<Foo>&& _foo, Foo* ptr) //(A)
Derived(unique_ptr<Foo> _foo, Foo* ptr) //(B) Since the move is cheap.
And suppose that it was called like this:
Derived(move(_foo), _foo.get()) //(1)
Derived{move(_foo), _foo.get()} //(2) @Artyer's idea
First, by adding a delegating constructor we essentially added a sequencing point that had to evaluate both the unique and raw pointer before passing them to Owner
and User
, respectively.
Second, all the arguments for a function call must be evaluated before the function is actually called, this order is unspecified for ()
and left-to-right for {}
(see the delete paragraph above).
Here lies the difference, if (B) is used, the copy has to be made at the call place and for ()
this copy can be made before assigning _foo.get()
to ptr
argument. For {}
it will always happen before. Thus setting ptr
incorrectly.
On the other hand, if (A) is chosen, only the r-value reference is copied into the argument and ptr
will always get the correct value no matter whether (1)
or (2)
is used.