Move constructor and vector of unique_ptr

Question

I tried to create simple struct holding references to some values of its parent. Parent is stored within unique_ptr, inside a vector. It's instantiated before being moved there. After the movement, references of course are no longer valid. I found a way to reinstantiate them, but I hate the solution (it's shown below). I thought that move constructor is called on collection.push_back(std::move(d)), but's it's not the case for Derived. It might be for unique_ptr though, I'm not sure about that. My question is - what should be preferred way of dealing with such situations? Is there a better solution that one I presented below? Would overriding a move constructor of unique_ptr help? Is this a good idea? Or, is this even a good idea to design objects in the way presented below?

#include <iostream> #include <vector> #include <memory> // Inner object of every Base instance, is used to keep reference to // Base's inner variables struct Ref { Ref(double &x, double &y) : x(x) , y(y) { } std::reference_wrapper<double> x; std::reference_wrapper<double> y; }; struct Point { double x; double y; }; struct Base { virtual ~Base() { } // every derived class uses this vector std::vector<Ref> refs; // some meaningless pure virtual method, ignore it virtual void draw() = 0; }; struct Derived : public Base { Derived() { std::cout << "Derived constructed" << std::endl; } // Method for adding point and relating it with // a reference in refs vector void add(double x, double y) { points.push_back({x, y}); refs.push_back( {points.back().x, points.back().y} ); } // some meaningless pure virtual method, ignore it virtual void draw() override { } // this vector is specific to this particular derived class std::vector<Point> points; }; int main() { // some vector for storing objects std::vector<std::unique_ptr<Base>> collection; { auto d = std::unique_ptr<Derived>(new Derived()); d->add(0.01, 0.02); d->add(1.111, 2.222); d->add(14.3333, 3.1414); collection.push_back(std::move(d)); } // posible solution (I hate it) { auto d = std::unique_ptr<Derived>(new Derived()); d->add(0.01, 0.02); d->add(1.111, 2.222); d->add(14.3333, 3.1414); collection.push_back(std::move(d)); auto c = dynamic_cast<Derived *>(collection.back().get()); for (int i = 0; i < c->points.size(); i++) { c->refs[i].x = c->points[i].x; c->refs[i].y = c->points[i].y; } } // Let's take 1st vector element and cast it to Derived { auto d = dynamic_cast<Derived *>(collection[0].get()); std::cout << "values from points vector:" << std::endl; // These work correctly after moving std::cout << d->points[0].x << std::endl; std::cout << d->points[0].y << std::endl; std::cout << d->points[1].x << std::endl; std::cout << d->points[1].y << std::endl; std::cout << d->points[2].x << std::endl; std::cout << d->points[2].y << std::endl; std::cout << "values from refs vector:" << std::endl; // References of course do not work anymore std::cout << d->refs[0].x << std::endl; std::cout << d->refs[0].y << std::endl; std::cout << d->refs[1].x << std::endl; std::cout << d->refs[1].y << std::endl; std::cout << d->refs[2].x << std::endl; std::cout << d->refs[2].y << std::endl; } // Let's take 2nd vector element and cast it to Derived { auto d = dynamic_cast<Derived *>(collection[1].get()); std::cout << "values from points vector:" << std::endl; // These work correctly after moving std::cout << d->points[0].x << std::endl; std::cout << d->points[0].y << std::endl; std::cout << d->points[1].x << std::endl; std::cout << d->points[1].y << std::endl; std::cout << d->points[2].x << std::endl; std::cout << d->points[2].y << std::endl; std::cout << "values from refs vector with ugly fix:" << std::endl; // References of course do not work anymore std::cout << d->refs[0].x << std::endl; std::cout << d->refs[0].y << std::endl; std::cout << d->refs[1].x << std::endl; std::cout << d->refs[1].y << std::endl; std::cout << d->refs[2].x << std::endl; std::cout << d->refs[2].y << std::endl; } return 0; }

Output:

Derived constructed Derived constructed values from points vector: 0.01 0.02 1.111 2.222 14.3333 3.1414 values from refs vector: 0 0.02 4.94602e-317 4.94603e-317 14.3333 3.1414 values from points vector: 0.01 0.02 1.111 2.222 14.3333 3.1414 values from refs vector with ugly fix: 0.01 0.02 1.111 2.222 14.3333 3.1414

Your references are getting invalidated by points.push_back({x,y});, not collection.push_back(std::move(d));. The latter calls a move constructor of unique_ptr, but it's still the same Derived object. — aschepler
– aschepler, Commented Apr 27, 2017 at 0:53
@aschepler thanks! I forgot about it - added. Wow, you're right about points.push_back({x,y});, no idea how I missed that, thank you! — solusipse
– solusipse, Commented Apr 27, 2017 at 1:10

Galik · Accepted Answer · 2017-04-27 01:31:30Z

According to the standard, references should not be invalidated by the move. The real problem is the std::vector::push_back which nvalidates everything if the capacity changes.

One solution is to use a std::deque because it never invalidates reference with push_back():

#include <iostream> #include <vector> #include <deque> #include <memory> struct Point { double x; double y; }; struct Base { // every derived class uses this vector std::vector<Point*> refs; // some meaningless pure virtual method, ignore it virtual ~Base() = default; virtual void draw() = 0; }; struct Derived : public Base { Derived() { std::cout << "Derived constructed" << std::endl; } // Method for adding point and relating it with // a reference in refs vector void add(double x, double y) { points.push_back({x, y}); refs.push_back(&points.back()); } // some meaningless pure virtual method, ignore it void draw() override { } // this vector is specific to this particular derived class std::deque<Point> points; }; int main() { // some vector for storing objects std::vector<std::unique_ptr<Base>> collection; { auto d = std::unique_ptr<Derived>(new Derived()); d->add(0.01, 0.02); d->add(1.111, 2.222); d->add(14.3333, 3.1414); collection.push_back(std::move(d)); // No ugly fix needed } // Let's take 1st vector element and cast it to Derived { auto d = dynamic_cast<Derived *>(collection[0].get()); std::cout << "values from points vector:" << std::endl; // These work correctly after moving std::cout << d->points[0].x << std::endl; std::cout << d->points[0].y << std::endl; std::cout << d->points[1].x << std::endl; std::cout << d->points[1].y << std::endl; std::cout << d->points[2].x << std::endl; std::cout << d->points[2].y << std::endl; std::cout << "values from refs vector:" << std::endl; // References still work std::cout << d->refs[0]->x << std::endl; std::cout << d->refs[0]->y << std::endl; std::cout << d->refs[1]->x << std::endl; std::cout << d->refs[1]->y << std::endl; std::cout << d->refs[2]->x << std::endl; std::cout << d->refs[2]->y << std::endl; } return 0; }

Output:

Derived constructed values from points vector: 0.01 0.02 1.111 2.222 14.3333 3.1414 values from refs vector: 0.01 0.02 1.111 2.222 14.3333 3.1414

@solusipse you can also pre allocate the vector using vector.reserve(). If you don't push_back past the capacity, references won't be invalidated.

M.M · Accepted Answer · 2017-04-27 01:07:54Z

This line:

 refs.push_back( {points.back().x, points.back().y} );

means the new entry in refs will refer to the members of the last entry in points.

But next time you do points.push_back, that may cause a vector reallocation which invalidates all the references already stored in refs.

If you really want to persist with vector<Refs> you will have to redesign your code to ensure that the lifetime of the objects being referred to exceeds the lifetime of the vector<Refs>.

The unique_ptr is a red herring.

Thanks, I wasn't aware of that. I'm going to use deque as Galik advised and refactor later.

Collectives™ on Stack Overflow

Move constructor and vector of unique_ptr

2 Answers 2

2 Comments

1 Comment

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

1 Comment

Related