141

What is the best method to go about passing a shared_ptr of a derived type to a function that takes a shared_ptr of a base type?

I generally pass shared_ptrs by reference to avoid a needless copy:

int foo(const shared_ptr<bar>& ptr);

but this doesn't work if I try to do something like

int foo(const shared_ptr<Base>& ptr);

...

shared_ptr<Derived> bar = make_shared<Derived>();
foo(bar);

I could use

foo(dynamic_pointer_cast<Base, Derived>(bar));

but this seems sub-optimal for two reasons:

  • A dynamic_cast seems a bit excessive for a simple derived-to-base cast.
  • As I understand it, dynamic_pointer_cast creates a copy (albeit a temporary one) of the pointer to pass to the function.

Is there a better solution?

Update for posterity:

It turned out to be an issue of a missing header file. Also, what I was trying to do here is considered an antipattern. Generally,

  • Functions that don't impact an object's lifetime (i.e. the object remains valid for the duration of the function) should take a plain reference or pointer, e.g. int foo(bar& b).

  • Functions that consume an object (i.e. are the final users of a given object) should take a unique_ptr by value, e.g. int foo(unique_ptr<bar> b). Callers should std::move the value into the function.

  • Functions that extend the lifetime of an object should take a shared_ptr by value, e.g. int foo(shared_ptr<bar> b). The usual advice to avoid circular references applies.

See Herb Sutter's Back to Basics talk for details.

Community
  • 1
  • 1
Matt Kline
  • 10,149
  • 7
  • 50
  • 87
  • 9
    Why do you want to pass a `shared_ptr`? Why no const-reference of bar? – ipc Nov 15 '12 at 18:08
  • 2
    Any `dynamic` cast is only needed for downcasting. Also, passing the derived pointer should work just fine. It'll create a new `shared_ptr` with the same refcount (and increase it) and a pointer to the base, which then binds to the const reference. Since you're already taking a reference, however, I don't see why you want to take a `shared_ptr` at all. Take a `Base const&` and call `foo(*bar)`. – Xeo Nov 15 '12 at 18:08
  • @Xeo: Passing the derived pointer (i.e. `foo(bar)`) doesn't work, at least in MSVC 2010. – Matt Kline Nov 15 '12 at 18:11
  • 1
    What do you mean by "obviously doesn't work"? The code compiles and behaves correctly; are you asking how to avoid creating a temporary `shared_ptr` to pass to the function? I'm fairly sure there's no way to avoid that. – Mike Seymour Nov 15 '12 at 18:11
  • @Fred not true, quite the contrary; there is no reason to pass by value as Herb Sutter says, see http://channel9.msdn.com/Shows/Going+Deep/C-and-Beyond-2011-Scott-Andrei-and-Herb-Ask-Us-Anything @ 4:34 – Seth Carnegie Nov 15 '12 at 18:18
  • Basically he says "copies are expensive, avoid them." And if you take a `const shared_ptr&`, you can make a non-`const` copy anyway. – Seth Carnegie Nov 15 '12 at 18:24
  • 1
    @Seth: I disagree. I think there is reason to pass a shared pointer by value, and there's very little reason to pass a shared pointer by reference (and all this without advocating unneeded copies). Reasoning here http://stackoverflow.com/questions/10826541/passing-shared-pointers-as-arguments/10826907#10826907 – R. Martinho Fernandes Nov 15 '12 at 18:49
  • @R.MartinhoFernandes yes, I was assuming we weren't asking whether or not to use shared pointers at all. I read your very good article "Need speed? Pass by value." and if you're making copies, pass it by value of course, and if not, pass by reference to the thing that's being pointed to. – Seth Carnegie Nov 15 '12 at 18:56
  • (That's not my article, btw, that's by Dave Abrahams) – R. Martinho Fernandes Nov 15 '12 at 19:01
  • which header file was missing? – peetonn Aug 04 '16 at 00:55
  • @peetonn This was years ago at this point, but I believe I forgot to include the header of one of the types I was wrapping in a `shared_ptr`. Since the `shared_ptr` template needs to know the size of the object it's wrapping, this causes issues. – Matt Kline Aug 04 '16 at 16:59
  • @MattKline ehm,...not sure about that - how come forward declarations with `shared_ptr` work if it needs to know the size of the object? – peetonn Aug 04 '16 at 19:03
  • Hmm. Forward declarations seem to work in some cases and not in others. Perhaps this has something to do with how C++ "lazily" implements templates - the compiler doesn't bother checking template code that isn't instantiated elsewhere. At any rate, 1) I'm speaking beyond my expertise, and 2) The code above is a poor practice to begin with. Functions that take ownership should take a copy of the `shared_ptr`, and functions that do not should just take a `const` reference. – Matt Kline Aug 04 '16 at 22:20

4 Answers4

80

This will also happen if you've forgotten to specify public inheritance on the derived class, i.e. if like me you write this:

class Derived : Base
{
};

Instead of:

class Derived : public Base
{
};
Felix
  • 3,783
  • 5
  • 34
  • 53
dshepherd
  • 4,989
  • 4
  • 39
  • 46
59

Although Base and Derived are covariant and raw pointers to them will act accordingly, shared_ptr<Base> and shared_ptr<Derived> are not covariant. The dynamic_pointer_cast is the correct and simplest way to handle this problem.

(Edit: static_pointer_cast would be more appropriate because you're casting from derived to base, which is safe and doesn't require runtime checks. See comments below.)

However, if your foo() function doesn't wish to take part in extending the lifetime (or, rather, take part in the shared ownership of the object), then its best to accept a const Base& and dereference the shared_ptr when passing it to foo().

void foo(const Base& base);
[...]
shared_ptr<Derived> spDerived = getDerived();
foo(*spDerived);

As an aside, because shared_ptr types cannot be covariant, the rules of implicit conversions across covariant return types does not apply when returning types of shared_ptr<T>.

Bret Kuhns
  • 4,034
  • 5
  • 31
  • 43
  • 49
    They're not covariant, but `shared_ptr` is implicitly convertible to `shared_ptr`, so the code should work with no casting shenanigans. – Mike Seymour Nov 15 '12 at 18:16
  • 11
    Um, `shared_ptr` has a constructor that takes a `shared_ptr` and does the appropriate conversion if `Ty*` is implicitly convertible to `Other*`. And if a cast is needed, `static_pointer_cast` is the appropriate one here, not `dynamic_pointer_cast`. – Pete Becker Nov 15 '12 at 18:17
  • True, but not with his reference parameter, as in the question. He would need to do a copy, regardless. But, if he's using refs to `shared_ptr`'s to avoid the reference count, then there's really no good reason to be using a `shared_ptr` in the first place. It's best to use `const Base&` instead. – Bret Kuhns Nov 15 '12 at 18:20
  • @PeteBecker See my comment to Mike about the conversion constructor. I honestly didn't know about `static_pointer_cast`, thanks. – Bret Kuhns Nov 15 '12 at 18:21
  • In http://www.cplusplus.com/reference/memory/static_pointer_cast/ it says: "If sp is not empty, the returned object shares ownership over sp's resources, increasing by one the use count." Isn't it possible to pass by reference *without* increasing the ref count? – Kim Homann Mar 14 '19 at 14:30
  • @KimHomann the reference count would not increase in the example from my answer. Passing the pointed-to object by reference (eg, `const Foobar&`) does not cause `std::shared_ptr` to affect the count. Using `static_pointer_cast` (or `dynamic_point_cast`), however, _will_ affect the reference count. The latter has to because it is participating in the lifetime of the object. – Bret Kuhns Apr 03 '19 at 20:13
  • @BretKuhns I have had a similar issue, and my derived class did not inherit public from the base class. Then the cast simply works. Also - I pass on a member object as a shared_ptr to a function as const ref to keep a reference, a perfectly reasonable scenario. – HankTheTank Sep 30 '19 at 13:33
  • As has been mentioned, you should be able to pass const shared_ptr& as const shared_ptr& no problem, I do this all the time everywhere without thinking. Only time I ever run into problems is when I do one of the two mistakes mentioned in other answers by dshepherd and Phil Rosenberg. This should not be the accepted answer as the information is either wrong or misleading – Iron Attorney Oct 26 '19 at 13:40
  • I do not know why this is the accepted answer. It is perfectly fine to convert from `shared_ptr` to `shared_ptr`, as most smart pointers define such conversion functions both for construction and assignment. To new comers first see if your reason is in one of [these](https://stackoverflow.com/a/46836519/59081) [answers](https://stackoverflow.com/a/51500756/59081). – Tanveer Badar Feb 09 '20 at 05:56
  • 1
    @TanveerBadar Not sure. Perhaps this failed to compile in 2012? (specifically using Visual Studio 2010 or 2012). But you're absolutely right, OP's code should absolutely compile if the full definition of a /publicly derived/ class is visible to the compiler. – Bret Kuhns Feb 10 '20 at 14:51
19

Also check that the #include of the header file containing the full declaration of the derived class is in your source file.

I had this problem. The std::shared<derived> would not cast to std::shared<base>. I had forward declared both classes so that I could hold pointers to them, but because I didn't have the #include the compiler could not see that one class was derived from the other.

Phil Rosenberg
  • 1,597
  • 1
  • 14
  • 22
  • 3
    Wow, I didn't expect it but this fixed it for me. I was being extra careful and only including header files where I needed them so some of them were only in source files and forward declaring them in headers as you said. – jigglypuff Jan 24 '19 at 02:49
  • 2
    Stupid compiler is stupid. This was my issue. Thank you! – Tanveer Badar Feb 09 '20 at 05:53
16

Sounds like you're trying too hard. shared_ptr is cheap to copy; that's one of its goals. Passing them around by reference doesn't really accomplish much. If you don't want sharing, pass the raw pointer.

That said, there are two ways to do this that I can think of off the top of my head:

foo(shared_ptr<Base>(bar));
foo(static_pointer_cast<Base>(bar));
Pete Becker
  • 74,985
  • 8
  • 76
  • 165
  • 11
    No, they're not cheap to copy, they should be passed by reference whenever possible. – Seth Carnegie Nov 15 '12 at 18:31
  • 1
    @SethCarnegie - do you have measurements to establish that its a bottleneck in your code? It's just an atomic integer increment. – Pete Becker Nov 15 '12 at 18:32
  • 2
    I'm just saying what Herb Sutter says. Watch http://channel9.msdn.com/Shows/Going+Deep/C-and-Beyond-2011-Scott-Andrei-and-Herb-Ask-Us-Anything and go to 4:34 – Seth Carnegie Nov 15 '12 at 18:33
  • 7
    @SethCarnegie - did Herb profile your code to see whether passing by value was a bottleneck? – Pete Becker Nov 15 '12 at 18:35
  • 1
    I trust Sutter more than you. You didn't profile my code either to determine it's _not_ a bottleneck, so it's you vs. him, and he wins. – Seth Carnegie Nov 15 '12 at 18:36
  • 32
    @SethCarnegie - that doesn't answer the question I asked. And, for what it's worth, I wrote the `shared_ptr` implementation that Microsoft ships. – Pete Becker Nov 15 '12 at 18:37
  • 1
    @PeteBecker: It has a definite cost, and should be avoided when possible. See [this thread](http://stackoverflow.com/questions/2502394/the-cost-of-passing-by-shared-ptr) – Matt Kline Nov 15 '12 at 18:37
  • 5
    @slavik262 - no, it **sometimes** should be avoided. Like any hand optimization, it should be done if you have evidence that it's a bottleneck in your program. If I pass half a dozen `shared_ptr` objects in two dozen places, there's no reason at all to complicate things by passing them by reference. – Pete Becker Nov 15 '12 at 18:39
  • 8
    @SethCarnegie - you've got the heuristic backwards. Hand optimizations should generally **not** be done **unless** you can show that they are needed. – Pete Becker Nov 15 '12 at 18:41
  • 2
    @PeteBecker (regarding you writing the MSVC implementation of `shared_ptr`) then I find it amazing that you don't concede to Herb Sutter knowing what you know. I did answer your question, just not with the answer you wanted. When it's something as trivial as passing by reference, it doesn't complicate things. Not passing by reference is premature pessimisation. Why pass stuff by value for the heck of it? Seriously, if someone like Sutter recommends something, you don't really change your mind unless someone else you respect more recommends the opposite, and that's not happening here. – Seth Carnegie Nov 15 '12 at 18:44
  • 22
    It's only "premature" optimization if you need to work at it. I see no problem in adopting efficient idioms over inefficient ones, whether it makes a difference in a particular context or not. – Mark Ransom Nov 15 '12 at 18:46
  • 1
    @SethCarnegie - if "it doesn't complicate things", why did the original poster have to ask how to do it? – Pete Becker Nov 15 '12 at 18:46
  • 4
    @PeteBecker "Hand optimizations should generally not be done unless you can show that they are needed." writing `const Foo&` instead of `std::shared_ptr` is certainly an optimization for my both hands. Seriously, if it takes less time to write, is more clear, AND is faster, it isn't an optimization, it is mandatory. – Jean-Michaël Celerier Aug 04 '16 at 09:45
  • 4
    @MarkRansom In this case, you *do* need to work at it, you have to prove that `foo` won't call anything that will implicitly cause `ptr` to be deleted, thus leading to AV, if the rest of `foo` attempts to use the smart-pointer-as-reference. (This was actually a problem in one codebase I've maintained, though to be fair, things might not have been "as tight" as they should have been). – Chris O Oct 20 '16 at 18:35
  • The performance degradation doesn't come at assignment time, but when the refcount is *decremented*. Please see this answer (not the accepted one) for a concise description: https://stackoverflow.com/questions/16966043/is-atomic-decrementing-more-expensive-than-incrementing/16967003#16967003 – Tom Mettam Mar 04 '19 at 14:55
  • It is recommended to pass on shared pointers as const reference. I'Ve seen a talk of the developers explaining that a shared_ptr is not different from any other object, so const reference is the way to go. – HankTheTank Sep 30 '19 at 13:36