12

Assume the following:

template<typename Item>
class Pipeline
{
    [...]

    void connect(OutputSide<Item> first, InputSide<Item> second)
    {
        Queue<Item> queue;
        first.setOutputQueue(&queue);
        second.setInputQueue(&queue);
        queues.push_back(std::move(queue));
    }

    [...]

    std::vector<Queue<Item> > queues;
};

Will the pointers to queue still work in "first" and "second" after the move?

Jan Stephan
  • 403
  • 3
  • 11

3 Answers3

13

Does std::move invalidate pointers?

No. An object still exists after being moved from, so any pointers to that object are still valid. If the Queue is sensibly implemented, then moving from it should leave it in a valid state (i.e. it's safe to destroy or reassign it); but may change its state (perhaps leaving it empty).

Will the pointers to queue still work in "first" and "second" after the move?

No. They will point to the local object that's been moved from; as described above, you can't make any assumptions about that object's state after the move.

Much worse than that is that when the function returns, it's destroyed, leaving the pointers dangling. They are now invalid, not pointing to any object, and using them will give undefined behaviour.

Perhaps you want them to point to the object that's been moved into queues:

queues.push_back(std::move(queue));
first.setOutputQueue(&queue.back());
second.setInputQueue(&queue.back());

but, since queues is a vector, those pointers will be invalidated when the queue next reallocates its memory.

To fix that problem, use a container like deque or list which doesn't move its elements after insertion. Alternatively, at the cost of an extra level of indirection, you could store (smart) pointers rather than objects, as described in Danvil's answer.

Mike Seymour
  • 249,747
  • 28
  • 448
  • 644
  • I don't understand why this answer got (-1). The provided explanation is ok, and the workaround (push, then get a pointer, not the other way) is valid, assuming that we read to the end and change the `queues` to a list-kind container. – quetzalcoatl Apr 14 '14 at 12:55
  • It would be nice if the downvoter could explain what's wrong. I'd like to correct anything I've got wrong here, but an unexplained downvote doesn't help me do that. – Mike Seymour Apr 14 '14 at 13:03
  • @MikeSeymour: I wish the first downvoter, at least, would comment on the reason of its downvote :( – Matthieu M. Apr 14 '14 at 13:21
  • I would like to choose this answer as well as it has been very helpful. Unfortunately I can choose only one and my reputation doesn't allow me to upvote your answer yet. You did clear things up for me, thank you. – Jan Stephan Apr 15 '14 at 20:05
7

The pointers will not work, because queue is a local object which will be deleted at the end of connect. Even by using std::move you still create a new object at a new memory location. It will just try to use as much as possible from the "old" object.

Additionally the whole thing will not work at all independent of using std::move as push_back possibly has to reallocate. Thus a call to connect may invalidate all your old pointers.

A possible solution is creating Queue objects on the heap. The following suggestion uses C++11:

#include <memory>

template<typename Item>
class Pipeline
{
    [...]

    void connect(OutputSide<Item> first, InputSide<Item> second)
    {
        auto queue = std::make_shared<Queue<Item>>();
        first.setOutputQueue(queue);
        second.setInputQueue(queue);
        queues.push_back(queue);
    }

    [...]

    std::vector<std::shared_ptr<Queue<Item>>> queues;
};
Danvil
  • 22,240
  • 19
  • 65
  • 88
  • Maybe check Seymour's answer. There's no reason (other than sticking to a vector!) in managing the shared pointer here. Just first push to a list, then use the newly added object. (edit) of course, always can get into lists-vs-vector discussion, but well, there are other containers.. – quetzalcoatl Apr 14 '14 at 12:56
  • @quetzalcoatl: It is only a suggestion to use `shared_ptr`. Personally I would prefer it over making assumptions on how a given container is implemented. – Danvil Apr 14 '14 at 12:59
  • It seems to me there is no reason to use a `shared_ptr` rather than a `unique_ptr` here; thus I would advise using `unique_ptr`. – Matthieu M. Apr 14 '14 at 13:20
2

Others provided nice and detailed explanations, but your question indicates that you do not understand fully what move does, or what it is designed to do. I'll try to describe it in simple words.

move, as the name implies, is meant to move things. But what can be moved? You cannot move an object once it is allocated somewhere. Recall the new moving-construtcor added recently, which resembles the copy-constructor..

So, it's all about the "contents" of an object. Both copy- and move-constructors are meant to operate on the "contents". So is the std::move. It is meant to move the contents of one object into the target, and in contrast to the copy, it is meant to leave no trace of contents in the original location.

It is meant to be used everywhere where something forces us to make a copy which we don't really care about and which we really'd like to actually omit, and only have the contents already in the target place.

That is, the usage of "move" indicates that a copy will be made, contents will be moved there, and original will be cleared (sometimes some steps might be skipped, but still, that's the basic idea of a move).

This clearly indicates that, even if the original survives, any pointers to the original object will not point to the destination, which received the content. At best, they will point to the original thing that was just 'cleared'. At worst, it will point to a completely unusable thing.

Now look at your code. It fills the queue and then takes the pointer to the original, then moves the queue.

I hope that's clear now what's happening and what you can do with it.

quetzalcoatl
  • 32,194
  • 8
  • 68
  • 107