Agreed. We should probably document this in the C++ developer docs.
On Mon, Feb 8, 2021 at 12:04 PM Antoine Pitrou <anto...@python.org> wrote: > > > Hi Micah, > > That's roughly my mental model as well. > > However, for 4) I would say that return a const ref to shared_ptr if > preferable because the caller will often need the ownership (especially > with Array, ArrayData, DataType, etc.). > > Regards > > Antoine. > > > Le 08/02/2021 à 18:02, Micah Kornfield a écrit : > > I'm not sure how consistent we are with how shared_ptr is used as a > > parameter to methods and as a return type. In reviewing and writing code > > I've been using these guidelines for myself and I was wondering if they > > align with others: > > > > 1. If a copy of a shared_ptr is not intended to be made by the method then > > use a const ref to underlying type. i.e. void Foo(const Array& array) is > > preferable to void Foo(const shared_ptr<Array>& array) [1]. > > > > 2. If a copy is always going to be made pass by value. i.e. void > > Foo(std::shared_ptr<Array>) and to std::move within the method. The last > > time I did research on this allowed for eliminating shared_ptr overhead if > > the caller also can std::move() the parameter. > > > > 3. If a copy might be made pass the shared_ptr by const reference. i.e. > > void > > Foo(const shared_ptr<T>& array) The exception to this if the contents of > > the shared_ptr a reference can effectively be copied cheaply without as is > > the case with Array via ArrayData in which case #1 applies. > > > > 4. For accessor methods prefer returning by const ref or underlying ref to > > underlying when appropriate. i.e. const std::shared_ptr<Array>& foo() > > or > > const Array& Foo(). > > > > 5. For factory like methods return a copy i.e. std::shared_ptr<Array> > > MakeFoo(); > > > > Is this other people's mental model? I'd like to update our style guide so > > we can hopefully drive consistency over time. > > > > Thanks, > > Micah > > > > [1] Array is somewhat of a special case because one can have essentially > > the same shared_ptr copy semantics by copying the underlying ArrayData > > object. > >
