[PATCH] unique_ptr with ability to get a raw pointer after release.

[Anton Yartsev]

On 25.09.2014 22:06, David Blaikie wrote:
>
>
> On Wed, Sep 24, 2014 at 7:15 PM, Антон Ярцев <anton.yartsev at gmail.com 
> <mailto:anton.yartsev at gmail.com>> wrote:
>
>     The discussion moved to llvmdev, topic "New type of smart pointer
>     for LLVM"
>
>     ================
>     Comment at: include/llvm/ADT/STLExtras.h:572
>     @@ +571,3 @@
>     +template<class T, class Deleter = SwitchControlledDeleter<T>>
>     +class unique_ptr_deferred_release : public std::unique_ptr<T,
>     Deleter> {
>     +public:
>     ----------------
>     dblaikie wrote:
>     > I'm sorry to go around on this a bit - but it'd be really good
>     to just discuss this design for a bit (maybe no llvm-dev for
>     increased visibility) as I believe we do have a need for a generic
>     "sometimes owns" pointer abstraction and I'm hesitant to build a
>     partial or awkward solution here. Some specific concerns (beyond
>     the vague one above) are:
>     >
>     > * inheritance from unique_ptr could be very subtle/surprising -
>     especially when hiding inherited functions. Taking a unique_ptr<T,
>     ...>& to this unique_ptr_deferred_release and then calling
>     release() provides different behavior.
>     >
>     > * what happens if release() is called twice? (should it be a
>     runtime failure? return null? not clear - maybe null return (as
>     you've implemented) makes sense)
>     >
>     > * there should probably be some way to both enter and exit this
>     domain with unique_ptr<T, NormalDeleterHere> - so both an implicit
>     ctor, and a "take()" perhaps, rather than the release(), comes
>     back to the "what to return when non-owning".
>     >
>     > * "deferred release" doesn't seem to connote the actual use case
>     here - that sounds like it might, say, "not release until the end
>     of its lifetime, even when released early" which doesn't make much
>     sense to me. This is really about conditional ownership - in some
>     cases the smart pointer may never own the object. Nothing is
>     deferred, it's just not handled here at all.
>     >
>     > I appreciate the work, and sorry to sort of cast doubt on it -
>     and I don't mean to dissuade you from prototyping as you see fit,
>     but I don't want to waste your time churning API possibilities if
>     you'd rather wait a bit, have a chat about what we're trying to
>     solve here, etc and then have a go at implementing (if you prefer
>     to have that discussion and iterate on the API design in-code,
>     that's fine too - but I don't want to annoy you by making you keep
>     reworking your code).
>     Thank you for looking at this. I agree with you, inheritance from
>     a unique_ptr is not a good idea and 'deferred release' and
>     'release()' ain't a good names. I'll start a new discussion at
>     llvm-dev.
>
>
> Thanks!
>
>
>     ================
>     Comment at: lib/TableGen/TGParser.cpp:235
>     @@ -234,3 +234,3 @@
>          // Clone the def and add it to the current multiclass
>     -    Record *NewDef = new Record(**i);
>     +    auto NewDef = make_unique<Record>(**i);
>
>     ----------------
>     dblaikie wrote:
>     > This change is independent, right? Just general cleanup change
>     that could be made today?
>     >
>     > (DefPrototypes should perhaps be a vector of unique_ptr, so that
>     last line in this block can be push_back(std::move(NewDef)); ?)
>     Yes, just a general cleanup, unique_ptr is sufficient here.
>
>
> I'd be happy for you to commit that for post-commit review at this 
> point, if you like. (or, if you don't have commit access, I can do 
> that for you)
Committed at r218462

>
>     http://reviews.llvm.org/D5443
>
>
>


-- 
Anton

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140926/97c844ea/attachment.html>