Is it good practice to bind shared pointers returned by functions to lvalue references to const?

Question

Although it took me a while to get used to it, I now grew the habit of letting my functions take shared pointer parameters by lvalue-reference to const rather than by value (unless I need to modify the original arguments, of course, in which case I take them by lvalue-reference to non-const):

void foo(std::shared_ptr<widget> const& pWidget)
//                               ^^^^^^
{
    // work with pWidget...
}

This has the advantage of avoiding an unnecessary copy of a shared pointer, which would mean thread-safely increasing the reference counting and potentially incurring in undesired overhead.

Now I've been wondering whether it is sane to adopt a somewhat symmetrical habit for retrieving shared pointers that are returned by value from functions, like at the end of the following code snippet:

struct X
{
    // ...
    std::shared_ptr<Widget> bar() const
    {
        // ...
        return pWidget;
    }
    // ...
    std::shared_ptr<Widget> pWidget;
};

// ...

// X x;
std::share_ptr<Widget> const& pWidget = x.bar();
//                     ^^^^^^

Are there any pitfalls with adopting such a coding habit? Is there any reason why I should prefer, in general, assigning a returned shared pointer to another shared pointer object rather than binding it to a reference?

Answer 1

This is just a remake of the old question of whether capturing a const reference to a temporary is more efficient than creating a copy. The simple answer is that it isn't. In the line:

// std::shared_ptr<Widget> bar();
std::shared_ptr<Widget> const & pWidget = bar();

The compiler needs to create a local unnamed variable (not temporary), initailize that with the call to bar() and then bind the reference to it:

std::shared_ptr<Widget> __tmp = bar();
std::shared_ptr<Widget> const & pWidget = __tmp;

In most cases it will avoid the creation of the reference and just alias the original object in the rest of the function, but at the end of the day whether the variable is called pWidget or __tmp and aliased won't give any advantage.

On the contrary, for the casual reader, it might look like bar() does not create an object but yield a reference to an already existing std::shared_ptr<Widget>, so the maintainer will have to seek out where bar() is defined to understand whether pWidget can be changed outside of the scope of this function.

Lifetime extension through binding to a const reference is a weird feature in the language that has very little practical use (namely when the reference is of a base and you don't quite care what the exact derived type returned by value is, i.e. ScopedGuard).