Task #1912
closed
Stop using const shared_ptr<T>& parameter type
Added by Junxiao Shi over 9 years ago.
Updated over 9 years ago.
Description
Some code is declaring const shared_ptr<T>& as parameter type.
This is potentially bad, because:
- It's risky to pass
shared_ptr by const reference, because there is no guarantee that the pointer would remain valid throughout the function call: the function may have a side effect of destroying the pointer. ref
- Passing by const reference instead of passing by value is a performance optimization, but this optimization is premature because it's not justified by benchmark.
We should stop adding new functions that declares const shared_ptr<T>& parameter type, and gradually change existing functions to pass shared_ptr by value.
Files
I disagree. I have checked on the issue before. The only "danger" is when you do circle assignments and this is not normal case in the first place.
We should use const& everywhere we are using right now and may be convert places where we don't.
Benchmark results on Ubuntu 12.04:
- passing by const reference: 7422122ns per million calls
- passing by value: 46806502ns per million calls
There is a performance benefit of 39ns per call.
The question is: is this small performance benefit worth sacrificing the long-term maintainability?
Code should be defensive when possible.
Yes, there is benefit and it is small only on surface. Also, there is no real harm to maintainability.
See this answer on why code using const shared_ptr<T>& would have maintainability issues: it relies on the calle not to decrement the reference count of the parameter, which cannot be guaranteed or proved in general case.
Sutter recommends a different approach, as explained here http://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/. The idea is basically:
- Don’t pass a smart pointer as a function parameter unless you want to use or manipulate the smart pointer itself, such as to share or transfer ownership.
- Prefer passing objects by value,
*, or &, not by smart pointer.
- Express that a function will store and share ownership of a heap object using a by-value
shared_ptr parameter.
- Use a
const shared_ptr<T>& as a parameter only if you’re not sure whether or not you’ll take a copy and share ownership; otherwise use T* instead (or if not nullable, a T&).
The whole article is worth a read.
I agree with the idea in note-6.
- Status changed from New to Rejected
Our code doesn't conform to Sutter's guidelines though. Why did you close this?
Our code doesn't conform to Sutter's guidelines though. Why did you close this?
This issue does not have a measurable goal, and thus is not constructive.
Future code review shall enforce the guidelines cited in note-6.
- Related to Task #2613: Eliminate unnecessary usage of shared_ptr in faces and channels added
- Related to deleted (Task #2613: Eliminate unnecessary usage of shared_ptr in faces and channels)
Also available in: Atom
PDF
Updated by 
Updated by 
Updated by Anonymous 
Updated by