Should I use std::shared pointer to pass a pointer

Question

Suppose I have an object which is managed by an std::unique_ptr. Other parts of my code need to access this object. What is the right solution to pass the pointer? Should I just pass the plain pointer by std::unique_ptr::get or should I use and pass an std::shared_ptr instead of the std::unique_ptr at all?

I have some preference for the std::unique_ptr because the owner of that pointer is actually responsible for cleanup. If I use a shared pointer, there's a chance that the object will remain alive due to a shared pointer even when it should actually be destroyed.

EDIT: Unfortunately, I forgot to mention that the pointer will not just be a an argument to a function call, but it will be stored in other objects to build up a network structure of objects. I do not prefer the shared pointer because then it's no longer clear, who actually owns the object.

Answer 1

Typically you would just pass a reference or plain pointer to other parts of the code that wish to observe the object.

Pass by reference:

void func(const Foo& foo);

std::unique_ptr<Foo> ptr;

// allocate ptr...

if(ptr)
    func(*ptr);

Pass by raw pointer:

void func(const Foo* foo);

std::unique_ptr<Foo> ptr;

// allocate ptr...

func(ptr.get());

The choice will depend on the need to pass a null pointer.

It is your responsibility to ensure by-design that observers do not use the pointer or reference after the unique_ptr has been destroyed. If you can't guarantee that then you must use a shared_ptr instead of a unique_ptr. Observers can hold a weak_ptr to indicate that they do not have ownership.

EDIT: Even if observers wish to hold on to the pointer or reference that is OK but it does make it more difficult to ensure it will not be used after the unique_ptr has been destroyed.

Answer 2

If the ownership of the managed object is not being transferred (and because it's a unique_ptr, ownership cannot be shared) then it's more correct to separate the logic in the called function from the concept of ownership. We do this by calling by reference.

This is a convoluted way of saying:

Given:

std::unique_ptr<Thing> thing_ptr;

to change the Thing:

// declaration
void doSomethingWith(Thing& thing); 

// called like this
doSomethingWith(*thing_ptr);

to use the Thing without modifying it.

// declaration
void doSomethingWith(const Thing& thing); 

// called like this
doSomethingWith(*thing_ptr);

The only time you'd want to mention the unique_ptr in the function signature would be if you were transferring ownership:

// declaration
void takeMyThing(std::unique_ptr<Thing> p);

// call site
takeMyThing(std::move(thing_ptr));

You never need to do this:

void useMyThing(const std::unique_ptr<Thing>& p);

The reason that this would be a bad idea is that if confuses the logic of useMyThing with the concept of ownership, thus narrowing the scope for re-use.

Consider:

useMyThing(const Thing& thing);

Thing x;
std::unique_ptr<Thing> thing_ptr = makeAThing();
useMyThing(x);
useMyThing(*thing_ptr);

Update:

Noting the update to the question - storing (non-owning) references to this object.

One way to do this is indeed to store a pointer. However, pointers suffer from the possibility of a logic error in that they can legally be null. Another problem with pointers is that they do not play nicely with std:: algorithms and containers - requiring custom compare functions and the like.

There is a std::-compliant way to do this - the std::reference_wrapper<>

So rather than this:

std::vector<Thing*> my_thing_ptrs;

do this:

std::vector<std::reference_wrapper<Thing>> my_thing_refs;

Since std::reference_wrapper<T> defines an operator T&, you can use the reference_wrapped object in any expression that would expect a T.

for example:

std::unique_ptr<Thing> t1 = make_thing();
std::unique_ptr<Thing> t2 = make_thing();
std::unique_ptr<Thing> t3 = make_thing();

std::vector<std::reference_wrapper<const Thing>> thing_cache;

store_thing(*t1);
store_thing(*t2);
store_thing(*t3);

int total = 0;
for(const auto& t : thing_cache) {
  total += value_of_thing(t);
}

where:

void store_thing(const Thing& t) {
  thing_cache.push_back(std::cref(t));
}

int value_of_thing(const Thing& t) {
  return <some calculation on t>;
}

Answer 3

This ground has already been covered by Herb Sutter in GotW #91 - Smart Pointer Parameters. It touches on the performance aspect as well, whilst the other answers, while great, focus on memory management.

Someone above recommended passing a smart pointer as const reference - changed his mind in later edit. We had code where this was abused - it made the signatures 'smarter' - i.e. more complicated but no benefit. The worst is when a junior developer takes over - he would not question as @Michael and 'learn' how to do it from a bad example. It works, but...

To me the smart pointer parameter passing matter is serious enough to be mentioned prominently in any book/article/post immediately after explaining what smart pointers are.

Wondering now if lint-like programs should flag passing by const reference as element of style.

Answer 4

Don't focus so much on the caller's requirements. What does the function need to be passed in? If it's only going to use the object for the duration of the call, then use a reference.

void func(const Foo& foo);

If it needs to retain ownership of the object past its duration, then pass in a shared_ptr

void func(std::shared_ptr<Foo> foo);

Answer 5

Since the question update, I'd prefer:

1. std::reference_wrapper<> as suggested by Richard Hodges, so long as you don't require the option of NULL values.

   Note that your use case does seem to require a default constructor though, so this is out unless you have some public static instance to use as default.

2. some custom not_your_pointer<T> type with the required semantics: default-constructable, copyable and assignable, convertible from unique_ptr and non-owning. Probably only worthwhile if you're using it a lot though, since writing a new smart pointer class requires some thought.

3. if you need to handle dangling references gracefully, use std::weak_ptr and change ownership to a shared_ptr (That is, only one shared_ptr exists: there's no ambiguity about ownership and object lifetime is unaffected)

---

Before the question update, I preferred:

1. indicate that ownership is not transferred by passing a reference:

   void foo(T &obj); // no-one expects this to transfer ownership
   

   called as:

   foo(*ptr);
   

   you do lose the ability to pass nullptr though, if that matters.

2. indicate that ownership is not transferred by explicitly prohibiting it:

   void foo(std::unique_ptr<T> const &p) {
       // can't move or reset p to take ownership
       // still get non-const access to T
   }
   

   this is clear, but does expose your memory management policy to the callee.

3. pass a raw pointer and document that ownership should not be transferred. This is the weakest and most error prone. Don't do it.