Passing smart-pointers by reference

Question

Smart-pointers are generally tiny so passing by value isn't a problem, but is there any problem passing references to them; or rather are there specific cases where this mustn't be done?

I'm writing a wrapper library and several of my classes wrap smart-pointer objects in the underlying library... my classes are not smart-pointers but the APIs currently pass smart-pointer objects by value.

e.g current code:

void class::method(const AnimalPtr pAnimal) { ... }

becomes

void class::method(const MyAnimal &animal){...}

where MyAnimal is my new wrapper class encapsulating AnimalPtr.

There is no guarantee the Wrapper classes won't one day grow beyond wrapping a smart-pointer, so passing by value makes me nervous.

Solution 1

You should pass shared pointers by reference, not value, in most cases. While the size of a std::shared_ptr is small, the cost of copying involves an atomic operation (conceptually an atomic increment and an atomic decrement on destruction of the copy, although I believe that some implementations manage to do a non-atomic increment).

In other cases, for example std::unique_ptr you might prefer to pass by value, as the copy will have to be a move and it clearly documents that ownership of the object is transferred to the function (if you don't want to transfer ownership, then pass a reference to the real object, not the std::unique_ptr).

In other cases your mileage might vary. You need to be aware of what the semantics of copy are for your smart pointer, and whether you need to pay for the cost or not.

Solution 2

It's ok to pass a smart pointer by reference, except if it's to a constructor. In a constructor, it's possible to store a reference to the original object, which violates the contract of the smart pointers. You would likely get memory corruption if you did that. Even if your constructor does not today store the reference, I would still be wary because code changes and it's an easy thing to miss if you decide later you need to hold the variable longer.

In a normal function, you cannot store a function parameter as a reference anywhere because references must be set during their initialization. You could assign the reference to some longer-living non-reference variable, but that would be a copy and so would increase its lifetime appropriately. So in either case, you could not hold onto it past when the calling function might have freed it. In this case, you might get a small performance boost with a reference, but I wouldn't plan on noticing it in most cases.

So I would say - constructor, always pass by value; other functions, pass by reference if you want.

Comments

[confusopoly]

In general storing references to smart pointers is not a good idea but passing a reference to it when you know it won't be stored is usually ok, see for example this question: stackoverflow.com/questions/179105/…

[Yakk - Adam Nevraumont]

Actually, I sort of like taking a std::unique_ptr<T> const& -- it implies that this is an owned object, and no, you can't have it. Similarly, std::unique_ptr<T>& means "this is an owned object, but you can change that ownership if you want to". On the other hand, I also like "pass-through unique_ptr", where you take the unique_ptr by value and return it by value in the return value...

[David Rodríguez - dribeas]

@Yakk: std::unique_ptr<T> const& forces your choice of smart pointer on your users and blocks calls with objects not managed by a std::unique_ptr unnecessarily, passing T& clearly implies that it is an owned (by someone, somewhere) object and is more flexible (besides enabling a level of const-correctness you cannot get with the const& to the smart pointer. Passing std::unique_ptr<T>& is unclear on the caller as of whether the object is still mine or yours, did you claim ownership? or can I use the pointer after the call?

[Chad]

You could create a copy of the pointed to object at anytime: Foo* f = smart_foo.get(), and you can't guarantee the lifetime of f.

[David Rodríguez - dribeas]

@Yakk: Yes, I am not talking about technically being able to detect it, but rather when writing the code. Does it make sense from a design perspective to have a function that might or might not claim ownership? Does it make sense to implement a function that calls it and that it has some post-call work to do on the object in some cases but that it is also fine not to do anything? I would find that code hard to get through a code review unless there is a very strong reason to do things in such a convoluted way

[JoshG79]

@chad - Yes, taking a pointer to a smart pointer is always unsafe. But the question was about references vs non-references, and this is no different for the two.

[Sam Harwell]

+1: When smart pointer are used throughout an application, the performance difference between passing by reference and passing by value becomes substantial (measurable and observable), especially in debug builds. You almost always want pass by const reference, and if you need to store a copy for later usage you can use the argument (passed by const reference) to initialize a new instance by value.