Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Reasons to return reference to std::unique_ptr

I wonder if there are any legitimate reasons to return unique pointers by reference in C++, i.e. std::unique_ptr<T>&?

I've never actually seen that trick before, but the new project I've got seems to use this pattern a lot. From the first glance, it just effectively breaks / circumvents the "unique ownership" contract, making it impossible to catch the error in compile-time. Consider the following example:

class TmContainer {
public:
    TmContainer() {
        // Create some sort of complex object on heap and store unique_ptr to it
        m_time = std::unique_ptr<tm>(new tm());
        // Store something meaningful in its fields
        m_time->tm_year = 42;
    }

    std::unique_ptr<tm>& time() { return m_time; }

private:
    std::unique_ptr<tm> m_time;
};

auto one = new TmContainer();
auto& myTime = one->time();
std::cout << myTime->tm_year; // works, outputs 42
delete one;
std::cout << myTime->tm_year; // obviously fails at runtime, as `one` is deleted

Note that if we'd returned just std::unique_ptr<tm> (not a reference), it would raise a clear compile-time error, or would force use to use move semantics:

// Compile-time error
std::unique_ptr<tm> time() { return m_time; }

// Works as expected, but m_time is no longer owned by the container
std::unique_ptr<tm> time() { return std::move(m_time); }

I suspect that a general rule of thumb is that all such cases warrant use of std::shared_ptr. Am I right?

like image 765
GreyCat Avatar asked Mar 05 '18 17:03

GreyCat


2 Answers

There are two use cases for this and in my opinion it is indicative of bad design. Having a non-const reference means that you can steal the resource or replace it without having to offer separate methods.

// Just create a handle to the managed object
auto& tm_ptr = tm_container.time();
do_something_to_tm(*tm_ptr);

// Steal the resource
std::unique_ptr<TmContainer> other_tm_ptr = std::move(tm_ptr);

// Replace the managed object with another one
tm_ptr = std::make_unique<TmContainer>;

I strongly advocate against these practices because they are error prone and less readable. It's best to offer an interface such as the following, provided you actually need this functionality.

tm& time() { return *m_time; }

std::unique_ptr<tm> release_time() { return {std::move(m_time)}; }

// If tm is cheap to move
void set_time(tm t) { m_time = make_unique<tm>(std::move(t)); }

// If tm is not cheap to move or not moveable at all
void set_time(std::unique_ptr t_ptr) { m_time = std::move(t_ptr); }
like image 186
patatahooligan Avatar answered Oct 06 '22 00:10

patatahooligan


This was too long of a comment. I don't have a good idea for requested use case. The only thing I imagine is some middle ware for a utility library.

The question is what do you want and need to model. What are the semantics. Returning reference does not anything useful that I know.

The only advantage in your example is no explicit destructor. If you wanted to make exact mirror of this code it'd be a raw pointer IMHO. What exact type one should use depends on the exact semantics that they want to model.

Perhaps the intention behind the code you show is to return a non-owning pointer, which idiomatically (as I have learned) is modeled through a raw pointer. If you need to guarantee the object is alive, then you should use shared_ptr but bear it mind, that it means sharing ownership - i.e. detaching it's lifetime from the TmContianer.

Using raw pointer would make your code fail the same, but one could argue, that there is no reason for explicit delete as well, and lifetime of the object can be properly managed through scopes.

This is of course debatable, as semantics and meaning of words and phrases is, but my experience says, that's how c++ people write, talk and understand the pointers.

like image 26
luk32 Avatar answered Oct 06 '22 00:10

luk32