[+cfe-dev] This conversation has already been happening on llvm-dev so there's no good way for me to capture the entire existing discussion (so I'm jumping you in part-way) & the subject line could be more descriptive, but I wanted to add Clang developers since many of the interesting cases of conditional ownership I've seen were in Clang. I know some of you are also on llvm-dev but not active readers, so it might be worth using this as a jumping off point to go & find the full llvm-dev thread, read that and, when replying, add cfe-dev. If anyone not on llvm-dev wants some more context there's the email archive here ( http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-September/thread.html#77136 ) and/or I'm happy to provide more context/summary myself. On Thu, Oct 2, 2014 at 9:44 AM, David Blaikie <dblaikie at gmail.com> wrote:> > > On Thu, Oct 2, 2014 at 2:43 AM, Anton Yartsev <anton.yartsev at gmail.com> > wrote: > >> Thanks for the feedback! >> >> >> On Wed, Oct 1, 2014 at 3:36 PM, David Blaikie <dblaikie at gmail.com> wrote: >> >>> On Wed, Oct 1, 2014 at 3:14 PM, Anton Yartsev <anton.yartsev at gmail.com> >>> wrote: >>> >>>> Ping! >>>> >>>> Suggested is a wrapper over a raw pointer that is intended for freeing >>>> wrapped memory at the end of wrappers lifetime if ownership of a raw >>>> pointer was not taken away during the lifetime of the wrapper. >>>> The main difference from unique_ptr is an ability to access the wrapped >>>> pointer after the ownership is transferred. >>>> To make the ownership clearer the wrapper is designed for local-scope >>>> usage only. >>>> >>> >>> I'm still concerned this isn't the right direction, and that we >>> instead want a more formal "maybe owning pointer". The specific use case >>> you've designed for here, in my experience, doesn't seem to come up often >>> enough to justify a custom ADT - but the more general tool of >>> sometimes-owning smart pointer seems common enough (& problematic enough) >>> to warrant a generic data structure (and such a structure would also be >>> appliable to the TGParser use case where this came up). >>> >> David, could you, please, clarify the concept of the "maybe owning >> pointer"? >> > > See my reply to Chandler with a list of classes that hold {T*, bool} > members where the bool indicates whether the T* needs to be deleted or not. > My original goal here was to provide an API to make those more robust (more > precisely: to remove the need to call "release()" and allow us to stay in a > clear(er) owning domain). > > >> >> >> >>> I'd love to hear some more opinions, but maybe we're not going to get >>> them... >>> >> >> I strongly agree that the example here isn't compelling. >> >> I think it is a very fundamental design problem when there is a need >> for a pointer value after it has been deallocated... >> >> Not deallocated but stored to the long-living storage. I agree, the >> problem is in design, the suggested wrapper is an intermediate solution, it >> was just designed to replace the existing ugly fixes. >> >> I even question whether we need a "maybe owning" smart pointer, or >> whether this is just an indication that the underlying data structure is >> *wrong*. The idea of "maybe" and "owning" going to gether, in any sense, >> seems flat out broken to me from a design perspective, and so I'm not >> enthused about providing abstractions that make it easier to build systems >> with unclear ownership semantics. >> >> >> -- >> Anton >> >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141008/2398b0c0/attachment.html>
& 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<T>}: LogDiagnosticPrinter::StreamOwner ASTUnit::ComputedPreamble::Owner On Wed, Oct 8, 2014 at 1:00 PM, David Blaikie <dblaikie at gmail.com> wrote:> [+cfe-dev] > > This conversation has already been happening on llvm-dev so there's no > good way for me to capture the entire existing discussion (so I'm jumping > you in part-way) & the subject line could be more descriptive, but I wanted > to add Clang developers since many of the interesting cases of conditional > ownership I've seen were in Clang. > > I know some of you are also on llvm-dev but not active readers, so it > might be worth using this as a jumping off point to go & find the full > llvm-dev thread, read that and, when replying, add cfe-dev. > > If anyone not on llvm-dev wants some more context there's the email > archive here ( > http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-September/thread.html#77136 > ) and/or I'm happy to provide more context/summary myself. > > > On Thu, Oct 2, 2014 at 9:44 AM, David Blaikie <dblaikie at gmail.com> wrote: > >> >> >> On Thu, Oct 2, 2014 at 2:43 AM, Anton Yartsev <anton.yartsev at gmail.com> >> wrote: >> >>> Thanks for the feedback! >>> >>> >>> On Wed, Oct 1, 2014 at 3:36 PM, David Blaikie <dblaikie at gmail.com> >>> wrote: >>> >>>> On Wed, Oct 1, 2014 at 3:14 PM, Anton Yartsev <anton.yartsev at gmail.com> >>>> wrote: >>>> >>>>> Ping! >>>>> >>>>> Suggested is a wrapper over a raw pointer that is intended for freeing >>>>> wrapped memory at the end of wrappers lifetime if ownership of a raw >>>>> pointer was not taken away during the lifetime of the wrapper. >>>>> The main difference from unique_ptr is an ability to access the >>>>> wrapped pointer after the ownership is transferred. >>>>> To make the ownership clearer the wrapper is designed for local-scope >>>>> usage only. >>>>> >>>> >>>> I'm still concerned this isn't the right direction, and that we >>>> instead want a more formal "maybe owning pointer". The specific use case >>>> you've designed for here, in my experience, doesn't seem to come up often >>>> enough to justify a custom ADT - but the more general tool of >>>> sometimes-owning smart pointer seems common enough (& problematic enough) >>>> to warrant a generic data structure (and such a structure would also be >>>> appliable to the TGParser use case where this came up). >>>> >>> David, could you, please, clarify the concept of the "maybe owning >>> pointer"? >>> >> >> See my reply to Chandler with a list of classes that hold {T*, bool} >> members where the bool indicates whether the T* needs to be deleted or not. >> My original goal here was to provide an API to make those more robust (more >> precisely: to remove the need to call "release()" and allow us to stay in a >> clear(er) owning domain). >> >> >>> >>> >>> >>>> I'd love to hear some more opinions, but maybe we're not going to get >>>> them... >>>> >>> >>> I strongly agree that the example here isn't compelling. >>> >>> I think it is a very fundamental design problem when there is a need >>> for a pointer value after it has been deallocated... >>> >>> Not deallocated but stored to the long-living storage. I agree, the >>> problem is in design, the suggested wrapper is an intermediate solution, it >>> was just designed to replace the existing ugly fixes. >>> >>> I even question whether we need a "maybe owning" smart pointer, or >>> whether this is just an indication that the underlying data structure is >>> *wrong*. The idea of "maybe" and "owning" going to gether, in any sense, >>> seems flat out broken to me from a design perspective, and so I'm not >>> enthused about providing abstractions that make it easier to build systems >>> with unclear ownership semantics. >>> >>> >>> -- >>> Anton >>> >>> >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141008/8eb3cdf3/attachment.html>
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. On Wed, Oct 8, 2014 at 1:01 PM, David Blaikie <dblaikie at gmail.com> wrote:> & 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<T>}: > > LogDiagnosticPrinter::StreamOwner > ASTUnit::ComputedPreamble::Owner > > On Wed, Oct 8, 2014 at 1:00 PM, David Blaikie <dblaikie at gmail.com> wrote: > >> [+cfe-dev] >> >> This conversation has already been happening on llvm-dev so there's no >> good way for me to capture the entire existing discussion (so I'm jumping >> you in part-way) & the subject line could be more descriptive, but I wanted >> to add Clang developers since many of the interesting cases of conditional >> ownership I've seen were in Clang. >> >> I know some of you are also on llvm-dev but not active readers, so it >> might be worth using this as a jumping off point to go & find the full >> llvm-dev thread, read that and, when replying, add cfe-dev. >> >> If anyone not on llvm-dev wants some more context there's the email >> archive here ( >> http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-September/thread.html#77136 >> ) and/or I'm happy to provide more context/summary myself. >> >> >> On Thu, Oct 2, 2014 at 9:44 AM, David Blaikie <dblaikie at gmail.com> wrote: >> >>> >>> >>> On Thu, Oct 2, 2014 at 2:43 AM, Anton Yartsev <anton.yartsev at gmail.com> >>> wrote: >>> >>>> Thanks for the feedback! >>>> >>>> >>>> On Wed, Oct 1, 2014 at 3:36 PM, David Blaikie <dblaikie at gmail.com> >>>> wrote: >>>> >>>>> On Wed, Oct 1, 2014 at 3:14 PM, Anton Yartsev <anton.yartsev at gmail.com >>>>> > wrote: >>>>> >>>>>> Ping! >>>>>> >>>>>> Suggested is a wrapper over a raw pointer that is intended for >>>>>> freeing wrapped memory at the end of wrappers lifetime if ownership of a >>>>>> raw pointer was not taken away during the lifetime of the wrapper. >>>>>> The main difference from unique_ptr is an ability to access the >>>>>> wrapped pointer after the ownership is transferred. >>>>>> To make the ownership clearer the wrapper is designed for local-scope >>>>>> usage only. >>>>>> >>>>> >>>>> I'm still concerned this isn't the right direction, and that we >>>>> instead want a more formal "maybe owning pointer". The specific use case >>>>> you've designed for here, in my experience, doesn't seem to come up often >>>>> enough to justify a custom ADT - but the more general tool of >>>>> sometimes-owning smart pointer seems common enough (& problematic enough) >>>>> to warrant a generic data structure (and such a structure would also be >>>>> appliable to the TGParser use case where this came up). >>>>> >>>> David, could you, please, clarify the concept of the "maybe owning >>>> pointer"? >>>> >>> >>> See my reply to Chandler with a list of classes that hold {T*, bool} >>> members where the bool indicates whether the T* needs to be deleted or not. >>> My original goal here was to provide an API to make those more robust (more >>> precisely: to remove the need to call "release()" and allow us to stay in a >>> clear(er) owning domain). >>> >>> >>>> >>>> >>>> >>>>> I'd love to hear some more opinions, but maybe we're not going to get >>>>> them... >>>>> >>>> >>>> I strongly agree that the example here isn't compelling. >>>> >>>> I think it is a very fundamental design problem when there is a need >>>> for a pointer value after it has been deallocated... >>>> >>>> Not deallocated but stored to the long-living storage. I agree, the >>>> problem is in design, the suggested wrapper is an intermediate solution, it >>>> was just designed to replace the existing ugly fixes. >>>> >>>> I even question whether we need a "maybe owning" smart pointer, or >>>> whether this is just an indication that the underlying data structure is >>>> *wrong*. The idea of "maybe" and "owning" going to gether, in any sense, >>>> seems flat out broken to me from a design perspective, and so I'm not >>>> enthused about providing abstractions that make it easier to build systems >>>> with unclear ownership semantics. >>>> >>>> >>>> -- >>>> Anton >>>> >>>> >>> >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141113/d6f77c6e/attachment.html>