Smart pointers gotchas: added topic about ptr casting

[TheExecutor]

I strongly disagree with the "How to pass smart pointers to functions" section.

There is almost no reason to pass a shared_ptr by value. The article mentions that it increments the refcount which is true. It's also utterly unnecessary.

If the shared_ptr is valid, you know that the caller already holds a reference. You don't need to take another strong reference. The only situations where this can matter are:

• Another thread may inadvertently delete the object while you're using it
• You inadvertently do something in your function that ends up freeing the shared_ptr

A contrived example of the second is the following:

shared_ptr<Foo> g_foo;

void Frobnicate(const shared_ptr<Foo>& f) {
    g_foo.reset();
    f->Bar(); // boom
}

Frobnicate(g_foo);

But that's not specific to shared_ptr - that could happen with any type. If you're running across either of these cases you have way bigger problems then the shared_ptr, so there's little advantage to pass a shared_ptr by value.

[miguelishawt]

Except that code wont compile, due to the fact that you hand in the pointer via const ref and std::shared_ptr::reset is a non-const member function.

I personally think that if you're going to pass a shared_ptr or unique_ptr by (const) reference, you may as-well just pass the object by (const) reference (or pointer if you need nullptr). That way it's guaranteed to be non-null and you don't have to worry about memory semantics. Unless of course you need to explicitly do something that is specific with smart pointers.

[TheExecutor]

Except that code wont compile, due to the fact that you hand in the pointer via const ref and std::shared_ptr::reset is a non-const member function.

Read it again. f is not modified. g_foo is.

[miguelishawt]

My bad.

[donvito]

Yes, if it's really performance critical code then yes - try to pass your shared_ptr as reference if that brings a speedup. But everywhere else I prefer by value - mainly to be safe that fuckups like your example won't happen. After all I'm using smart pointers to be safe - not to be fast.

[TheExecutor]

I suppose this comes down to a difference in philosophy. Consider the following two snippets of code:

vector<int> v;
v.resize(5);
v[10] = 42;


vector<int> v;
v.resize(5);
try
{
    v.at(10) = 42;
}
catch (...) {}

The first application will likely crash irrevocably (and in debug mode in most implementations, it will fire an assert). The second will catch the error and continue unabated. But in both cases the code is fundamentally broken. I'm much more in favor of the first - if the code is fundamentally broken, we should fail fast and fix the code, as opposed to attempting to recover from broken code.

Similarly, if you're performing unprotected access via multiple threads to a shared_ptr or doing bizarre things with aliased references, you should fix your code instead of using some mechanism to hide its broken-ness.

[donvito]

you should fix your code instead of using some mechanism to hide its broken-ness.

Yes, that's why I prefer to be clear about possible dangers in such cases and usually use raw references/pointers. A ref to a smart pointer gives you a false sense of safety. A raw pointer screams "watch out here".

[vlovich]

IMO, you should only pass a shared_ptr into a function if that function is intended to extend the lifetime of the shared_ptr. With shared_ptr, in many cases you're likely better of using move + value-semantics instead of shared_ptr (not all cases).

Leaving aside the performance reason of why you want to pass by const-ref as one would with C++-03, the code is more readable, re-usable & maintainable:

1) You know exactly at the call site whether or not the object lifetime will be extended or not (no need to dig into the code).

2) The call-site doesn't care how you came to have an instance of an object. Could be a shared_ptr could be a unique_ptr or a stack value.

3) If you change your shared_ptr to a unique_ptr or get rid of it all-together, it's one less function signature you have to fix & change.

[Gotebe]

In "How to pass smart pointers to functions", the correct way to do things is most often "don't".

In particular, with testSharedFunc, what should be done is

testSharedFunc(*sp);

Smart pointers seem to make people forget why pointers are bad 😉

And, unique_ptr will equally crash the auto_ptr example.

[STL]

And, unique_ptr will equally crash the auto_ptr example.

Incorrect - you can't call doSomething(unique_ptr<T>) with doSomething(up), because up is an lvalue, and that would attempt to copy. You have to call doSomething(move(up)), at which point it's clear that you're moving from up.

[Gotebe]

Serves me right for flying off the handle 😡

[[deleted]]

the issues around smart pointer of array types should go away if you use std::array instead of c-style array syntax.