40

I have a function defined as follows:

void foo(std::shared_ptr<X> x) { ... };

If I declare a shared ptr to X:

std::shared_ptr<X> sourcePtr(new X(...));

I can then call foo as follows:

foo(std::move(sourcePtr));

or

foo(sourcePtr);

I understand that if I use the first option then sourcePtr becomes null. Does it also prevent the reference count from being incremented?

If that doesn't matter, which option should I prefer? Should I be considering anything else when making such a decision?

ksl
  • 4,519
  • 11
  • 65
  • 106
  • 5
    In short, you're right in that the reference counter will not be incremented. However I fail to see a practical use for it, unless you want to force the pointer you pass should be emptied, in which case you could have used `std::unique_ptr` instead. You shouldn't really see the new smart pointers as *pointer*, instead you should look at them from a *resource ownership* perspective. Can a resource be owned by only a single entity (`std::unique_ptr`) or by multiple entities (`std::shared_ptr`). – Some programmer dude Apr 15 '15 at 07:28
  • This is very similar question, although I am not sure if exact duplicate: http://stackoverflow.com/questions/10953325/should-i-stdmove-a-shared-ptr-in-a-move-constructor. this question is about passing arguments to function - in this way different... – PiotrNycz Apr 15 '15 at 07:31
  • 1
    @JoachimPileborg It's a simplified example of some test code, which doesn't tell the whole story but allows me to focus the question specifically on what I want to know. `Foo` needs to take a `shared_ptr` in my application but the test code doesn't need `sourcePtr` after the call to it. – ksl Apr 15 '15 at 07:33

1 Answers1

47

Yes, if you move the shared pointer into the function, then:

  1. the original sourcePtr will become null, and

  2. the reference count does not get modified.

If you know that you will no longer need the value of sourcePtr after the function call, moving it into the function is a slight optimisation, as it saves an atomic increment (and later decrement, when sourcePtr goes out of scope).

However, be careful that the identifier sourcePtr is still valid for the rest of the scope, just that it holds a null pointer. Which means the compiler will not complain if you use it after the move, but if you forget it was moved from, you'll in all likelihood dereference the null. I tend to use this "optimising" move a lot, and I've also been bitten by it a few times: more functionality is added to the function, and if you forget to undo the move, you get a nice crash.

So moving when you no longer need it is a slight optimisation coupled with a slight maintenance burden. It's up to you to weigh which is more important in your case.

The above assumes that there is code which actually uses sourcePtr between its declaration and the final call to foo (thanks to @WhozCraig for pointing it out). If there is not, you'd be much better off creating the pointer right at the call site:

foo(std::make_shared<X>(...));

This way, you save the same amount of atomic operations, and you don't have a potentially dangerous empty shared pointer lying around.

Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
  • 6
    I think that's pointless. That's why `shared_ptr` was introduced - not only because of safety (in terms of avoiding memory leaks), but also because expensive-to-copy objects can be safely shared for (almost) free. Optimizing single atomic ops sounds like a waste of time. – Mateusz Grzejek Apr 15 '15 at 07:36
  • 6
    Just for my own clarity, if the creation and invoke were indeed as simple as the example (and it may not be), then wouldn't `foo(std::make_shared(...));` accomplish the same thing without leaving an abandon (and as you point out, potentially risky) `std::shared_ptr` shell laying around? Certainly its not an option if there is intermediate `sourcePtr->member()` code between the two (the allocation and the call). But if there isn't? – WhozCraig Apr 15 '15 at 07:48
  • 1
    While I agree with others that this kind of optimization defeats the purpose of `shared_ptr`, this is the first post I've found that discusses the maintainability issue of moving vs. copying pointers (+1). In many cases you don't want references to the same resource all over the place for the same reasons you don't want global variables. If you have dynamic binding you still need pointers, specifically `unique_ptr`. Then you have to use `move` (more code), but you get better control of your resources and a slight performance gain in return. – sigvaldm Nov 06 '18 at 13:27