Moving a unique pointer - undefined behavior on cppreference?

Question

I have a follow-up question to this one: Move unique_ptr: reset the source vs. destroy the old object

For a quick summary of the original question, there is this sample code on cppreference:

struct List
{
    struct Node
    {
        int data;
        std::unique_ptr<Node> next;
    };
 
    std::unique_ptr<Node> head;
 
    ~List()
    {
        // destroy list nodes sequentially in a loop, the default destructor
        // would have invoked its `next`'s destructor recursively, which would
        // cause stack overflow for sufficiently large lists.
        while (head)
            head = std::move(head->next);
    }
 
    ...
};

The answer tells me that the loop in the destructor is well defined as far as the contained raw pointer goes, because unique_ptr::operator=(unique_ptr&& other) is defined to call reset(other.release()), which guarantees that the raw pointer is extracted from other before the raw_pointer held by this is deleted.

I believe that the code on cppreference is still wrong, because of the deleter, which is transferred after the call to reset deletes the unique_ptr whos reference we're working with. To clarify, a commented implementation of operator= as I understand it:

auto& unique_ptr<T, Deleter>::operator=(unique_ptr<T, Deleter>&& other)
{
    // Here, "other" refers to a unique_ptr that lives in the T that is owned by our instance
    // Thus, if we delete the raw pointer held in this instance, we delete the object that "other" references

    reset(other.release()); // This correctly transfers over the raw pointer from "other", but deletes the raw pointer held in this instance, thus deleting the object "other" refers to

    get_deleter() = std::forward<Deleter>(other.get_deleter()); // this should be UB, because "other" is a dangling reference
}

Is my reasoning correct? Is this actually UB and the code on cppref is wrong? Or is this okay because the unique_ptrs used have default deleters are not stored?

Answer 1

As far as I can tell you are correct. The current standard draft specifies the single-object std::unique_ptr's move assignment like this, see [unique.ptr.single.asgn]/3:

constexpr unique_ptr& operator=(unique_ptr&& u) noexcept;

[...]

Effects: Calls reset(u.release()) followed by get_deleter() = std​::​forward<D>(u.get_deleter()).

You are correct that it is possible that reset will indirectly end the lifetime of u as is the case in the linked list example. Then u.get_deleter() always has undefined behavior, regardless of the type of the deleter, since you can't call a non-static member function outside the lifetime of the object. That the managed pointer was released from u earlier by u.release() doesn't matter at all.

Therefore head = std::move(head->next); in the linked list example has undefined behavior.

And it seems that is indeed how all big three standard library implementations implement it as can be verified with C++23 where std::unique_ptr is constexpr-usable. The program

#include<memory>

struct A {
    std::unique_ptr<A> a;
};

consteval void f() {
    auto a = std::make_unique<A>(std::make_unique<A>());
    a = std::move(a->a);
}

int main() {
    f();
}

fails to compile with Clang libstdc++, Clang libc++ and MSVC due to access to an object outside its lifetime in a constant expression. (GCC seems to be more lenient on the UB in the constant expression. It is unspecified whether UB in library functions is detected, so that is fine.)

It may be possible that I misunderstand how "Effects:" is supposed to be interpreted in the standard, but this seems like a defect to me. This behavior is very unexpected. I think it should be possible to specify this in a way that works, e.g. by moving u to a temporary copy first, and then swapping or move-assigning by the current rules. This can also be used as a workaround (here with C++23 auto syntax):

head = auto(std::move(ahead->next));

Or if I am misunderstanding the meaning of the "Effects:" clause and it requires implementations to not have UB even if u would be deleted through reset, then all three major implementations are not behaving conforming.

I could not find any open LWG issue on this matter though.