c++c++14unique-ptrrvalue-referenceownership-semantics

Pass ownership of an object into method of the same object?


I came across come C++ code similar to the following (more or less minimal) example. Please consider the marked method call in the function on the bottom:

#include <memory>

static unsigned returnValue = 5;
void setReturnValue(unsigned u) { returnValue = u; }

class MyObject
{
    public:
        MyObject(unsigned uIN) : u(uIN) {}
        ~MyObject() { u = 42; }

        void method(std::unique_ptr<MyObject> uniqPtrToObject)
        {
            // Do something fancy with this unique pointer now, 
            // which will consume the object and its data
            setReturnValue(uniqPtrToObject->getValue());
        }
        unsigned getValue() { return u; }

    private:
        unsigned u;    // Some relevant object data
};

std::unique_ptr<MyObject> GetUniqToObject(unsigned u)
{
    // Get the object in a fancy way. For now, assume it has just been constructed.
    return std::make_unique<MyObject>(u);
}

int main()
{
    std::unique_ptr<MyObject> uniqPtrToObject = GetUniqToObject(0);

    // ===================================================
    // Oops!
    uniqPtrToObject->method(std::move(uniqPtrToObject));
    // ===================================================

    return returnValue;
}

It looks really strange to move away an object while calling one of its methods.

I already tried my example on https://godbolt.org (x64-86 gcc 12.2), and the program returned 0 as it "should". Just some fortunate coincidence?

I still wonder:

  1. Is this valid C++ syntax?
  2. If so, is it portable?
  3. If so, is it moral?

As I haven't found a good resource where to start research on this situation, I hope you have some advice where to start reading or how to decide this situation easily.


Solution

  • Since you tagged this question C++14, that means we now have to engage with the question of which expression resolves first: the uniqPtrToObject-> one or the initialization of the function parameter? The answer is... it is indeterminate. Neither is sequenced before the other in C++14. The -> is sequenced before the function call and its parameter evaluations in C++17, but this was an explicit change.

    That means that in C++14, your code's behavior is effectively undefined. If the initialization of the unique_ptr parameter happens before evaluating the -> expression, then uniqPtrToObject will have been moved from by the time -> has been evaluated. Thus, -> gets a nullptr and you access the state of a nullptr.

    Now, there is some evaluation order. But you don't know what it is, and compilers aren't required to tell you or even be consistent from one execution to the next. So it is unreliable.

    So let's assume we're talking about C++17. That makes your code well-defined, since it ensures that uniqPtrToObject-> gets evaluated first.

    However, even if your code "works", it is almost certainly useless within the context of this example. The reason being that you passed uniqPtrToObject by value. That means method() will destroy the unique_ptr, which in turn destroys the object it owns. So if the object stored some data internally, that data is now lost. Whatever modifications were done by this object are no longer accessible, unless it was stored in state outside of the ownership reach of the object.

    So even on C++ versions where this has consistently-defined behavior, it's not a good idea to actually do.