How to return smart pointers (shared_ptr), by reference or by value?

Question

Let's say I have a class with a method that returns a shared_ptr.

What are the possible benefits and drawbacks of returning it by reference or by value?

Two possible clues:

• Early object destruction. If I return the shared_ptr by (const) reference, the reference counter is not incremented, so I incur the risk of having the object deleted when it goes out of scope in another context (e.g. another thread). Is this correct? What if the environment is single-threaded, can this situation happen as well?
• Cost. Pass-by-value is certainly not free. Is it worth avoiding it whenever possible?

Thanks everybody.

Answer 1

Return smart pointers by value.

As you've said, if you return it by reference, you won't properly increment the reference count, which opens up the risk of deleting something at the improper time. That alone should be enough reason to not return by reference. Interfaces should be robust.

The cost concern is nowadays moot thanks to return value optimization (RVO), so you won't incur a increment-increment-decrement sequence or something like that in modern compilers. So the best way to return a shared_ptr is to simply return by value:

shared_ptr<T> Foo()
{
    return shared_ptr<T>(/* acquire something */);
};

This is a dead-obvious RVO opportunity for modern C++ compilers. I know for a fact that Visual C++ compilers implement RVO even when all optimizations are turned off. And with C++11's move semantics, this concern is even less relevant. (But the only way to be sure is to profile and experiment.)

If you're still not convinced, Dave Abrahams has an article that makes an argument for returning by value. I reproduce a snippet here; I highly recommend that you go read the entire article:

Be honest: how does the following code make you feel?

std::vector<std::string> get_names();
...
std::vector<std::string> const names = get_names();

Frankly, even though I should know better, it makes me nervous. In principle, when get_names() returns, we have to copy a vector of strings. Then, we need to copy it again when we initialize names, and we need to destroy the first copy. If there are N strings in the vector, each copy could require as many as N+1 memory allocations and a whole slew of cache-unfriendly data accesses > as the string contents are copied.

Rather than confront that sort of anxiety, I’ve often fallen back on pass-by-reference to avoid needless copies:

get_names(std::vector<std::string>& out_param );
...
std::vector<std::string> names;
get_names( names );

Unfortunately, this approach is far from ideal.

• The code grew by 150%
• We’ve had to drop const-ness because we’re mutating names.
• As functional programmers like to remind us, mutation makes code more complex to reason about by undermining referential transparency and equational reasoning.
• We no longer have strict value semantics for names.

But is it really necessary to mess up our code in this way to gain efficiency? Fortunately, the answer turns out to be no (and especially not if you are using C++0x).