Do we really need to ensure giving up ownership when moving a `unique_ptr`?

Question

I'm reading Nicolai M. Josuttis's C++ Move Semantics - The Complete Guide book (which is pretty good imho) and I'm not sure I agree with the comments in one of the examples.

Quote (from 6.1.2 - Guaranteed States of Moved-From Objects):

Similar code can be useful to release memory for an object that a unique pointer uses:

draw(std::move(up));  // the unique pointer might or might not give up ownership
up.reset();           // ensure we give up ownership and release any resource

Let's assume that the up variable is indeed unique_ptr and the draw function receives the unique_ptr by value (otherwise, what's the point of moving the pointer to a "passed-by-ref" function).

I understand that it is legal to call reset on a "moved-from" object. But what I do not understand is why it is "required" in order to "ensure we give up ownership and release any resource" and how is it possible that "the unique pointer might or might not give up ownership"?

After all, unique_ptrs cannot be copied and the whole idea is that they guarantee only one ownership.

So, afaik, if my two assumptions are correct, there is no need to call the reset function to ensure the ownership was given away.

Am I missing something?

Outfight

No, we don't need to care of that. – Fourdimensional

If draw takes an rvalue reference, you don't know for sure if it accepts ownership, or just takes a look and ignores the pointer. – Superman

@Superman Nice idea. Any reason for draw to take rvalue reference only to "take a look"? – Outfight

For a function called draw, not really. But in general a function might throw an exception before it has completed its work. Then, who knows? – Superman

If draw takes rvalue reference only to take a look, it's not following GotW#91. But that's just GotW#91 as best practices. Maybe the coding convention being used in some code was (in my opinion) not best practices... but is still perfectly legal code (awkwardness aside). – Frightened

draw might take ownership conditionally. (E.g., "If there is nothing to draw, then ignore the parameter since we have no use for it.") The reset forces the resource to be released if draw chose not to consume it. – Selfsupporting

Move construction and move assignment already transfer the ownership. That reset is redundant – Extravagancy

It will at least conceptually move ownership. After the move the orignal object must be left in an unspecified but valid state (so it can be destructed). Note std::move doesn't really do anything directly. So it is indeed to the internals of draw what do with it (though I must admit I find the fact that the code needs documenting quite surprising, it would be more consistent for draw to always take ownership. So bad example) – Standard

@Extravagancy No, the actual move should happen inside the implementation of draw and that implementation is not shown here. Imagine draw has NO implementation, then no move is performed (std::move itself doesn't move). But yeah IMO such an implementation violates the "no surprises" guideline. – Standard

Answer 1

First of all, if the parameter is accepted by value, then we have the guarantee that it's actually being moved from, not just maybe, and that ownership over resources is given up (unless the move constructor does nothing, but that would be nonsensical). We might have to call .reset() otherwise for two possible reasons:

You may have to .reset() if the argument isn't always moved from

Consider the following signature:

void draw(std::unique_ptr<T> &&uptr);

Such a signature is generally preferred over accepting moved-from parameters by value by CppCoreGuidelines F.18: For “will-move-from” parameters, pass by X&& and std::move the parameter, although an exception is made for types like std::unique_ptr.

In such a case, we don't know whether draw moves from the uptr for sure unless we look at its implementation:

void draw(std::unique_ptr<T> &&uptr) {
    // If 'condition' is false, then 'uptr' won't be moved from, and the
    // caller may have to .reset() to free up resources immediately.
    if (condition) {
        process_further(std::move(uptr));
    }
}

You may have to .reset() for swapping move assignment

Furthermore, a few people tend to implement the move assignment operator as follows:

T& operator=(T&& other) noexcept {
    this->swap(other);
    return *this;
}

void draw(std::unique_ptr<T> &&uptr) {
    // This effectively swaps 'something' with 'uptr', which means that
    // the caller has to .reset() to free the resources in 'something',
    // unless they can rely on the destructor of 'uptr' to do that.
    something = std::move(uptr);
}

This forces the caller to use .reset() if they want these resources freed immediately, not when the argument goes out of scope.

No standard library types implement move assignment like this, and it's overall not a good idea, but it is valid, and may force us to make the call.

Higgler