New type of smart pointer for LLVM

& repeating for the benefit of cfe-dev, here’s the list of known cases of conditional ownership (I’m sure there are others that don’t fit the exact naming convention I grep’d for):

In Clang, simply grepping for “Owns” comes up with the following boolean members:

CodeGenAction::OwnsVMContext
ASTReader::OwnsDeserializationListener
Diagnostic::OwnsDiagClient
Preprocessor::OwnsHeaderSearch
TokenLexer::OwnsTokens
Action::OwnsInputs (this ones trickier - it’s a boolean that indicates whether all the elements of a vector<T*> are owned or unowned)
ASTUnit::OwnsRemappedFileBuffers
VerifyDiagnosticConsumer::OwnsPrimaryClient
TextDiagnosticPrinter::OwnsOutputStream
FixItRewriter::OwnsClient
Tooling::OwnsAction

Some in LLVM:

circular_raw_ostream::OwnsStream
Arg::OwnsValues (another tricky one with a bool flag and a vector of raw pointers, if I recall correctly)

And a couple that I changed {T*, bool} to {T*, unique_ptr}:

LogDiagnosticPrinter::StreamOwner
ASTUnit::ComputedPreamble::Owner

Ping - we’ve hit another of these (propagating Diagnostic::OwnsDiagClient into other places) in

http://llvm.org/viewvc/llvm-project?view=revision&revision=221884

Any ideas how we should be tackling this overall? I’m not entirely convinced these are fixable by design and I think we might honestly want a conditional-ownership smart pointer…

But I’m happy to hold off on that a while longer if we’re going to make a concerted effort to avoid propagating these half-baked solutions and actually try to remove them when we come up against problems with/around them.

Could we consider moving the things you listed to shared pointer semantics ? It will be a simple and clear model; unless someone justifies that it will be a performance concern to use shared pointers there I don’t think we need a new and more complex to reason about smart pointer.

Could we consider moving the things you listed to shared pointer semantics
? It will be a simple and clear model; unless someone justifies that it
will be a performance concern to use shared pointers there I don’t think we
need a new and more complex to reason about smart pointer.

I'd generally prefer conditional ownership over shared ownership if
possible - it's a narrower contract & I can still think about where the
single owner is.

I know in at least some of these uses, shared pointer semantics would not
be applicable - TextDiagnosticPrinter::OwnsOutputStream, for example either
owns its own newly allocated stream or uses std::cout (or cerr, or
something) - it can never share the ownership of that stream, so it really
must be "own something or own nothing". (I suppose we could use a custom
no-op deleter on a shared_ptr in that case, though)

But I'm open to the idea that that's the simpler/better answer than
introducing a new construct - just a bit hesitant. Thanks for bringing it
up as a possibility.

- David

I’m not so sure. With unique_ptr and shared_ptr you know exactly what is the ownership, without needing to know where it came from, it is very clear.
With conditional ownership I will have to hunt around in the codebase and find the trail between different code paths for where the pointer came from, so that I know who owns it and in what conditions.

I'd generally prefer conditional ownership over shared ownership if
possible - it's a narrower contract & I can still think about where the
single owner is.

I’m not so sure. With unique_ptr and shared_ptr you know exactly what is
the ownership, without needing to know where it came from, it is very clear.

Except that won't be true here - in at least some of these cases of
conditional ownership, at the point where we enter into this arrangement we
may not have ownership of the thing at all (it may've been passed down
through several levels of non-owning, then we're calling into an API that
has conditional ownership - or it may be a concrete object (stack or
global) that cannot be shared) - if we used shared_ptr we could lie about
it by creating a shared_ptr with a null deleter, which in some ways
restricts the weirdness to where it's happening, but could be more
confusing to developers rather than less (hey, I had this shared_ptr and
somehow the object was destroyed - how could that ever happen? At least if
it's conditional ownership they'll have a chance to realize that someone
else is failing to live up to their side of the bargain)

I agree that lying with shared_ptr is bad; I’m advocating that we refactor the API and callers chain to use a real shared_ptr unless there is a strong reason not to.
If there is some rare ugly part that this is not possible we could debate it specifically and we may just live with the ugliness, but I’d prefer that we don’t make such unclear owning semantics easier to propagate.