Is it okay to "Move" an object from a queue, if you're about to pop from it?

Question

I've been working on a parser for commands (which are fancy wrappers around large arrays of data), and have a queue that unhandled commands reside on. If I need a command, I query it with code like this:

boost::optional<command> get_command() {
    if (!has_command()) return boost::optional<command>(nullptr);
    else {
        boost::optional<command> comm(command_feed.front()); //command_feed is declared as a std::queue<command>
        command_feed.pop();
        return comm;
    }
}

The problem is, these commands could be megabytes in size, under the right circumstances, and need to parse pretty quickly. My thought was that I could optimize the transferal to a move like so:

boost::optional<command> get_command() {
    if (!has_command()) return boost::optional<command>(nullptr);
    else {
        boost::optional<command> comm(std::move(command_feed.front())); //command_feed is declared as a std::queue<command>
        command_feed.pop();
        return comm;
    }
}

And it seems to work for this specific case, but can this be used as a general purpose solution to any properly maintained RAII object, or should I be doing something else?

Solution 1

Yes, this is perfectly safe:

std::queue<T> q;
// add stuff...

T top = std::move(q.front());
q.pop();

pop() doesn't have any preconditions on the first element in the q having a specified state, and since you're not subsequently using q.front() you don't have to deal with that object being invalidated any more.

Sounds like a good idea to do!

Solution 2

It depends on what the move constructor for your type does. If it leaves the original object in a state that can safely be destroyed, then all is well. If not, then you may be in trouble. Note that the comments about preconditions and valid states are about constraints on types defined in the standard library. Types that you define do not have those constraints, except to the extent that they use types from the standard library. So look at your move constructor to sort out what you can and can't do with a moved-from object.

Solution 3

Yes. As long as your std::queue's container template argument ensures that there are no preconditions on the state of its contained values for pop_front(); the default for std::queue is std::deque and that offers the guarantee.

As long as you ensure what I wrote on the previous paragraph, you are completely safe. You're about to remove that item from your queue, thus there is no reason not to move it out since you are taking ownership of that object.

Solution 4

moving an object may leave it in an invalid state. It's invariants are no longer guaranteed. You would be safe popping it from a non-intrusive queue.

• The std::move itself does nothing other than tell the compiler, that it can select a comm routine that takes an r-value.

• A well written comm routine, would then steal the representation from the old object for the new object. For instance, just copy the pointers to the new object, and zero the pointers in the old object (that way the old object destructor won't destroy the arrays).

• if comm is not overloaded to do this there will not be any benefit to std::mov.

View more solutions

Comments

[Steve Lorimer]

Yes, moving it is safe, so long as the only thing you then do with it is pop it

[Revolver_Ocelot]

@SteveLorimer or reassign it. Or do any other operation without preconditions.

[Ilya Popov]

@Revolver_Ocelot Assignment is not necessarily without preconditions.

[jterm]

These precautions apply to any time an object is moved. Unrelated to the queue pop.

[juanchopanza]

It is perfectly safe... depending on the characteristics of T. So it isn't guaranteed to be safe.

[Barry]

@juanchopanza Sure, as long as T is MoveConstructible.

[juanchopanza]

That is the requirement I had in mind. The point if that it has to be in a certain state after being moved from.

[Ilya Popov]

@juanchopanza The standard already requires that moved-from object should be in a valid state (at least destructible), so there is no problem here.

[Ilya Popov]

If the moved-from object is not destructible, this move constructor/assignment it is basically unusable.

[juanchopanza]

@IlyaPopov Do you have a reference for that? I don't remember that being the case in C++11, but it may have been changed / fixed.

[Ilya Popov]

I checked now and I agree it is vague. For library types, It is guaranteed in [lib.types.movedfrom]. For other types, in the definition of MoveConstructible [moveconstructible] in posconditions it says: "rv’s state is unspecified [Note:rv must still meet the requirements of the library component that is using it. The operations listed in those requirements must work as specified whether rv has been moved from or not. — end note ]"

[Ilya Popov]

I read this note as: If the type command is used in a queue, then both moved-from or not it must meet the queue's requirements.

[anderas]

moving an object may leave it in an invalid state. It's invariants are no longer guaranteed. <- this is considered bad practice. Moving from an object should leave it in a valid but unspecified state!

[Rotartsi]

Hmm does this apply to priority_queues too? You have to cast away const from priority_queue::top()

[Barry]

@TheCppZoo The question is about std::queue, which does not have a top. For std::priority_queue, you could need to const_cast, yes (which would also be safe).