5 ms·

I disagree with this advice; I pass shared_ptr by const ref all the time to avoid lots of useless ref count updates. I mean, I guess it's a way to avoid this p

I disagree with this advice; I pass shared_ptr by const ref all the time to avoid lots of useless ref count updates. I mean, I guess it's a way to avoid this particular problem (https://godbolt.org/z/8g6HJg https://godbolt.org/z/8g6HJg), but even without passing shared_ptr by const ref that code still has a far worse code smell, namely the way it mixes manual and automated memory management. After all, the whole point of creating a shared_ptr instance is to take (possibly shared) ownership of a thing.

reducesuffering 7y ago
What the article skips over, but is somewhat implied, is that in the case you want to pass const std::shared_ptr<Widget>&, you're not taking ownership anymore. So you should be dereferencing (outside the func rather than in) and passing const Widget& instead, because the shared_ptr isn't useful anymore and you're "breaking" the contract of shared_ptr managing it's memory. const Widget& makes it clearer you're not owning the memory in the func. There is a caveat that is mentioned in other comments about there's still a use for const std::shared_ptr<Widget>& if you're not sure whether you'll copy the Widget to a shared_ptr in the func.
- mark-r 7y ago
  I'm pretty sure making a copy of a shared_ptr needs to modify it, so having a const reference doesn't work.
  - tlb 7y ago
    It doesn't modify anything inside the shared_ptr itself, but a refcount which is pointed to by a pointer field in the shared_ptr. So you can say foo(shared_ptr<T> const &x) { saved_x = x; } and it'll update the refcount exactly once when doing the assignment.
    - mark-r 7y ago
      That's the problem with modern C++, you can't reason about it on general principles - you have to know way too much about implementation detail.
codeflo 7y ago
You limit the generality of the function by passing it more than it needs. If your function doesn’t care about the shared_ptr itself, then you should only be passing a const ref to the value (which you get by dereferencing the shared_ptr at the call site). That way, your function works with values in shared_ptrs, unique_ptrs, values on the stack, manually managed pointers etc., without you doing any extra work.
- usefulcat 7y ago
  This comment is exactly right, and is what I do whenever possible. However there are cases where a function may or may not need to copy a shared_pointer that was passed to it as an argument. In such cases, the extra refcount updates that would be incurred for passing by value are not useful but still incur a cost. So my usual practice is to copy a shared_ptr only at the point where ownership needs to be shared.

reducesuffering 7y ago

What the article skips over, but is somewhat implied, is that in the case you want to pass const std::shared_ptr<Widget>&, you're not taking ownership anymore. So you should be dereferencing (outside the func rather than in) and passing const Widget& instead, because the shared_ptr isn't useful anymore and you're "breaking" the contract of shared_ptr managing it's memory. const Widget& makes it clearer you're not owning the memory in the func. There is a caveat that is mentioned in other comments about there's still a use for const std::shared_ptr<Widget>& if you're not sure whether you'll copy the Widget to a shared_ptr in the func.

mark-r 7y ago
I'm pretty sure making a copy of a shared_ptr needs to modify it, so having a const reference doesn't work.
- tlb 7y ago
  It doesn't modify anything inside the shared_ptr itself, but a refcount which is pointed to by a pointer field in the shared_ptr. So you can say foo(shared_ptr<T> const &x) { saved_x = x; } and it'll update the refcount exactly once when doing the assignment.
  - mark-r 7y ago
    That's the problem with modern C++, you can't reason about it on general principles - you have to know way too much about implementation detail.

codeflo 7y ago

You limit the generality of the function by passing it more than it needs. If your function doesn’t care about the shared_ptr itself, then you should only be passing a const ref to the value (which you get by dereferencing the shared_ptr at the call site). That way, your function works with values in shared_ptrs, unique_ptrs, values on the stack, manually managed pointers etc., without you doing any extra work.

usefulcat 7y ago
This comment is exactly right, and is what I do whenever possible. However there are cases where a function may or may not need to copy a shared_pointer that was passed to it as an argument. In such cases, the extra refcount updates that would be incurred for passing by value are not useful but still incur a cost. So my usual practice is to copy a shared_ptr only at the point where ownership needs to be shared.