<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 25.09.2014 22:06, David Blaikie
wrote:<br>
</div>
<blockquote
cite="mid:CAENS6EuQHSWgMsEXOEzDgvQRgmfg1Xw5c0gqTXUBd_LNDXxWmA@mail.gmail.com"
type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Wed, Sep 24, 2014 at 7:15 PM,
Антон Ярцев <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:anton.yartsev@gmail.com" target="_blank">anton.yartsev@gmail.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">The
discussion moved to llvmdev, topic "New type of smart
pointer for LLVM"<br>
<span class=""><br>
================<br>
Comment at: include/llvm/ADT/STLExtras.h:572<br>
@@ +571,3 @@<br>
+template<class T, class Deleter =
SwitchControlledDeleter<T>><br>
+class unique_ptr_deferred_release : public
std::unique_ptr<T, Deleter> {<br>
+public:<br>
----------------<br>
</span><span class="">dblaikie wrote:<br>
> 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:<br>
><br>
> * 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.<br>
><br>
> * 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)<br>
><br>
> * 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".<br>
><br>
> * "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.<br>
><br>
> 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).<br>
</span>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.<br>
</blockquote>
<div><br>
</div>
<div>Thanks!</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
================<br>
Comment at: lib/TableGen/TGParser.cpp:235<br>
@@ -234,3 +234,3 @@<br>
// Clone the def and add it to the current
multiclass<br>
- Record *NewDef = new Record(**i);<br>
+ auto NewDef = make_unique<Record>(**i);<br>
<br>
----------------<br>
</span><span class="">dblaikie wrote:<br>
> This change is independent, right? Just general
cleanup change that could be made today?<br>
><br>
> (DefPrototypes should perhaps be a vector of
unique_ptr, so that last line in this block can be
push_back(std::move(NewDef)); ?)<br>
</span>Yes, just a general cleanup, unique_ptr is
sufficient here.<br>
</blockquote>
<div><br>
</div>
<div>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)</div>
</div>
</div>
</div>
</blockquote>
Committed at r218462<br>
<br>
<blockquote
cite="mid:CAENS6EuQHSWgMsEXOEzDgvQRgmfg1Xw5c0gqTXUBd_LNDXxWmA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<a moz-do-not-send="true"
href="http://reviews.llvm.org/D5443" target="_blank">http://reviews.llvm.org/D5443</a><br>
<br>
<br>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="72">--
Anton</pre>
</body>
</html>