I like it. I'd make the recommendation more formal:
"Never use raw pointers except for passing references to objects that might be nullptr, and only if the receivers lifetime is strictly shorter than that of the caller. Use unique_ptr unless the object is shared between multiple libprocess processes, each of which has a different lifetime; then use process::Shared". We also need to fix Try to work with unique_ptr in this case, but we should do that anyway. On Mon, Dec 15, 2014 at 1:59 PM, Jie Yu <[email protected]> wrote: > > I have a proposal. Please let me know if that makes sense to you guys. > > One downside of using Owned is that it introduces a dependency to > libprocess (see my previous reply). Since semantically, Owned and > unique_ptr should be the same. I am in favor of replacing Owned with > unique_ptr (using standard lib as much as possible). > > One of the reason we want to keep Owned previously is because it allows > transformation between Owned and Shared. And we do want to keep Shared > because it enforces const access to underlying object (which is not > provided by shared_ptr). > > What I am proposing right now is: > 1) Get rid of Owned and replace it with unique_ptr > 2) allow transformation between unique_ptr and Shared > > In that way, we can consistently use unique_ptr (common case) without > introducing libprocess dependencies. And, we still keep Shared for > enforcing const access (used by replicated log). > > What do you guys think? > > - Jie > > On Mon, Dec 15, 2014 at 1:31 PM, Dominic Hamon <[email protected]> > wrote: > > > > I built some test cases using unique_ptr and ran into some issues pretty > > much immediately. > > > > Everywhere I wanted to replace was not a raw pointer T* but a Try<T*>. > The > > factory methods that were used to create these already return a Try<T*>, > > which means we'd need to change those to return Try<std::unique_ptr<T>> > > which is intrusive. Further, Try itself doesn't readily support a > > std::unique_ptr as a template parameter due to using heap allocations and > > copy constructors internally. > > > > I think Jie's idea to use unrestricted union may help here though we'll > > have to wait for g++-4.6. > > > > > > On Fri, Dec 12, 2014 at 3:01 PM, Dominic Hamon <[email protected]> > wrote: > > > > > > Great idea. > > > > > > There is one place that immediately springs to mind that I think is > > > trivial but will get us used to the idea and exercise some of the > demons: > > > Unit tests. Within a test we regularly create some object on the heap > and > > > then delete it at the end of the test, or for the lifetime of a test > > > fixture. All of these should be well scoped in lifetime and are prime > > > targets for unique_ptr. > > > > > > On Wed, Dec 10, 2014 at 3:32 PM, Jie Yu <[email protected]> wrote: > > >> > > >> Dominic, > > >> > > >> Thank you for brining this up! Instead of making a decision based on > > high > > >> level design and coding philosophy, I would really love to see some > > >> concrete examples in our code base where we can start to use > unique_ptr > > >> and > > >> why it's better. Thoughts? > > >> > > >> - Jie > > >> > > >> On Wed, Dec 10, 2014 at 3:20 PM, Dominic Hamon < > [email protected] > > > > > >> wrote: > > >> > > >> > Hello! > > >> > > > >> > We have access to std::unique_ptr, as many of you know, but we're > not > > >> using > > >> > it. I'd like to start using it. Before we do, I'd like to discuss > how > > we > > >> > use it, when we use it, and things to be aware of. > > >> > > > >> > The style guide has the following to say (long quote, sorry): > > >> > > > >> > " > > >> > Prefer to have single, fixed owners for dynamically allocated > objects. > > >> > Prefer to transfer ownership with smart pointers. > > >> > > > >> > "Ownership" is a bookkeeping technique for managing dynamically > > >> allocated > > >> > memory (and other resources). The owner of a dynamically allocated > > >> object > > >> > is an object or function that is responsible for ensuring that it is > > >> > deleted when no longer needed. Ownership can sometimes be shared, in > > >> which > > >> > case the last owner is typically responsible for deleting it. Even > > when > > >> > ownership is not shared, it can be transferred from one piece of > code > > to > > >> > another. > > >> > > > >> > "Smart" pointers are classes that act like pointers, e.g. by > > overloading > > >> > the * and -> operators. Some smart pointer types can be used to > > automate > > >> > ownership bookkeeping, to ensure these responsibilities are met. > > >> > std::unique_ptr is a smart pointer type introduced in C++11, which > > >> > expresses exclusive ownership of a dynamically allocated object; the > > >> object > > >> > is deleted when the std::unique_ptr goes out of scope. It cannot be > > >> copied, > > >> > but can be moved to represent ownership transfer. std::shared_ptr > is a > > >> > smart pointer type that expresses shared ownership of a dynamically > > >> > allocated object. std::shared_ptrs can be copied; ownership of the > > >> object > > >> > is shared among all copies, and the object is deleted when the last > > >> > std::shared_ptr is destroyed. > > >> > > > >> > It's virtually impossible to manage dynamically allocated memory > > without > > >> > some sort of ownership logic. > > >> > Transferring ownership of an object can be cheaper than copying it > (if > > >> > copying it is even possible). > > >> > Transferring ownership can be simpler than 'borrowing' a pointer or > > >> > reference, because it reduces the need to coordinate the lifetime of > > the > > >> > object between the two users. > > >> > Smart pointers can improve readability by making ownership logic > > >> explicit, > > >> > self-documenting, and unambiguous. > > >> > Smart pointers can eliminate manual ownership bookkeeping, > simplifying > > >> the > > >> > code and ruling out large classes of errors. > > >> > For const objects, shared ownership can be a simple and efficient > > >> > alternative to deep copying. > > >> > Ownership must be represented and transferred via pointers (whether > > >> smart > > >> > or plain). Pointer semantics are more complicated than value > > semantics, > > >> > especially in APIs: you have to worry not just about ownership, but > > also > > >> > aliasing, lifetime, and mutability, among other issues. > > >> > The performance costs of value semantics are often overestimated, so > > the > > >> > performance benefits of ownership transfer might not justify the > > >> > readability and complexity costs. > > >> > APIs that transfer ownership force their clients into a single > memory > > >> > management model. > > >> > Code using smart pointers is less explicit about where the resource > > >> > releases take place. > > >> > std::unique_ptr expresses ownership transfer using C++11's move > > >> semantics, > > >> > which are relatively new and may confuse some programmers. > > >> > Shared ownership can be a tempting alternative to careful ownership > > >> design, > > >> > obfuscating the design of a system. > > >> > Shared ownership requires explicit bookkeeping at run-time, which > can > > be > > >> > costly. > > >> > In some cases (e.g. cyclic references), objects with shared > ownership > > >> may > > >> > never be deleted. > > >> > Smart pointers are not perfect substitutes for plain pointers. > > >> > If dynamic allocation is necessary, prefer to keep ownership with > the > > >> code > > >> > that allocated it. If other code needs access to the object, > consider > > >> > passing it a copy, or passing a pointer or reference without > > >> transferring > > >> > ownership. Prefer to use std::unique_ptr to make ownership transfer > > >> > explicit. For example: > > >> > > > >> > std::unique_ptr<Foo> FooFactory(); > > >> > void FooConsumer(std::unique_ptr<Foo> ptr); > > >> > Do not design your code to use shared ownership without a very good > > >> reason. > > >> > One such reason is to avoid expensive copy operations, but you > should > > >> only > > >> > do this if the performance benefits are significant, and the > > underlying > > >> > object is immutable (i.e. std::shared_ptr<const Foo>). If you do use > > >> shared > > >> > ownership, prefer to use std::shared_ptr. > > >> > > > >> > Do not use scoped_ptr in new code unless you need to be compatible > > with > > >> > older versions of C++. Never use std::auto_ptr. Instead, use > > >> > std::unique_ptr. > > >> > " > > >> > > > >> > Now I'm a big fan of explicit ownership and moving ownership rather > > than > > >> > sharing non-smart pointers, but I recognise that in our code-base, > > >> > ownership is difficult to reason about in many cases. However, we do > > >> have > > >> > quite a few cases where we manage lifetime scope with explicit > delete > > >> calls > > >> > and I'd like to start by eliminating those. Ie, using > std::unique_ptr > > to > > >> > manage lifetime not necessarily ownership. > > >> > > > >> > This is difficult though as we may pass these pointers to other > > methods > > >> or > > >> > even other libprocess processes. In cases that we can reason about > the > > >> > lifetime of the various pointers, that should be fine, but we have > to > > be > > >> > careful. > > >> > > > >> > One option is to start by replacing these pointers with > > process::Owned. > > >> The > > >> > downside to this approach is that it introduces more non-standard > > types > > >> > and, because Owned is implemented using std::shared_ptr, doesn't > move > > us > > >> > closer to defining clear ownership. > > >> > > > >> > So the floor is open. Do we: > > >> > > > >> > 1) embrace std::unique_ptr, eliminate raw pointers except in rare, > > >> > well-defined cases from a lifetime point of view? > > >> > 2) eliminate raw pointers in favour of std::shared_ptr and > > >> std::unique_ptr > > >> > then work to eliminate the std::shared_ptrs > > >> > 3) use process::Owned everywhere and then find places where we can > > >> > transition them to std::unique_ptr > > >> > 4) do nothing; there's no great benefit to std::unique_ptr and this > > >> smart > > >> > pointer thing is just a fad. > > >> > > > >> > > > >> > - dominic > > >> > > > >> > > > >> > -- > > >> > Dominic Hamon | @mrdo | Twitter > > >> > *There are no bad ideas; only good ideas that go horribly wrong.* > > >> > > > >> > > > > > > > > > -- > > > Dominic Hamon | @mrdo | Twitter > > > *There are no bad ideas; only good ideas that go horribly wrong.* > > > > > > > > > -- > > Dominic Hamon | @mrdo | Twitter > > *There are no bad ideas; only good ideas that go horribly wrong.* > > > -- Dominic Hamon | @mrdo | Twitter *There are no bad ideas; only good ideas that go horribly wrong.*
