<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 25, 2014 at 11:06 AM, Ben Pope <span dir="ltr"><<a href="mailto:benpope81@gmail.com" target="_blank">benpope81@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">On Friday, September 26, 2014 01:11 AM, David Blaikie wrote:<br>
</span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">
<br>
<br>
On Thu, Sep 25, 2014 at 1:44 AM, Renato Golin <<a href="mailto:renato.golin@linaro.org" target="_blank">renato.golin@linaro.org</a><br></span><span class="">
<mailto:<a href="mailto:renato.golin@linaro.org" target="_blank">renato.golin@linaro.<u></u>org</a>>> wrote:<br>
<br>
On 25 September 2014 06:16, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a><br></span><span class="">
<mailto:<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>>> wrote:<br>
> I can go & dredge up some examples if we want to discuss the particular<br>
> merits & whether each of those cases would be better solved in some other<br>
> way, but it seemed pervasive enough in the current codebase that some<br>
> incremental improvement could be gained by replacing these cases with a more<br>
> specific tool for the job. We might still consider this tool to be "not<br>
> ideal".<br>
<br>
Having done that in another code base, I can see both merits and<br>
problems.<br>
<br>
Our smart pointer behaved more or less like it's described above<br>
(explicit acquire/release)<br>
<br>
<br>
I'm not quite sure I follow you (or that you're following me). The goal<br>
isn't about being explicit about acquire/release (indeed I wouldn't mind<br>
more implicit acquisition from an always-owning pointer (unique_ptr) to<br>
a sometimes-owning pointer (whatever we end up calling it), though<br>
release is always going to be explicit I suspect).<br>
</span></blockquote>
<br>
Given the example, the smart_ptr can't deal with ownership at all, it merely destroys the pointee in the face of exceptions or forgetfulness (which, in the example, can't really be tolerated).<span class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
and we've seen around 20% performance<br>
improvement over shared_ptr due to savings on acquire/release<br>
semantics.<br>
<br>
<br>
The existing use cases I'm interested in aren't using shared_ptr and<br>
wouldn't be ideally migrated to it (in some cases the existing ownership<br>
might be on the stack (so it can't be shared)<br>
</blockquote>
<br></span>
null_deleter?<span class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
or several layers up the<br>
call stack through which raw pointers have been passed (eg: build<br>
something, pass it through a few APIs, underlying thing 'wants' to take<br>
ownership, but it will be constructed/destroyed before this API returns<br>
- so we hack around it either by having a "OwnsThing" flag in the<br>
underlying thing, or having a "takeThing" we hope we call before the<br>
underlying thing dies and destroys the thing)).<br>
</blockquote>
<br></span>
Obviously combing the ownership flag and the pointer into some kind of "smart type" would be preferable, since they must be dealt with together.<span class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
We're dealing with types that have raw pointer + bool indicating<br>
ownership members or worse, types which take ownership but we lie to<br>
about giving them ownership (so some API takes a non-owning T* (or T&),<br>
passes it as owning to some other thing, then is sure to take ownership<br>
back from that thing and call 'release()" on the unique_ptr to ensure<br>
ownership was never really taken).<br>
</blockquote>
<br></span>
Confusing. null_deleter, again?<span class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
We had three derivative types of pointers<br>
(Shared/Owned/Linked) which would just differ on the default behaviour<br>
of construction / destruction, but all of them could still explicitly<br>
call getClear / getLink / etc.<br>
<br>
The downside was that almost every interaction with smart pointers had<br>
to be carefully planned and there was a lot of room for errors,<br>
<br>
<br>
My hope is that having a single construct for conditional ownership will<br>
be less confusing than the existing ad-hoc solutions in many places.<br>
It's possible that the right answer is to remove the conditional<br>
ownership in these APIs entirely, but I'm not sure that's<br>
possible/practical/desired (it might be - which is why I've been<br>
hesitant to write this abstraction myself just yet - sort of letting it<br>
stew in my head a bit to see what feels right).<br>
</blockquote>
<br></span>
If the code really does look like the example, hesitation is probably wise.<span class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
especially from people that didn't know the context. In the end, the<br>
amount of work that had to be put to make it work was similar than<br>
when dealing with normal pointers,<br>
<br>
<br>
I rather hope that's not the case - these conditionally owning raw<br>
pointers are pretty subtle, easy to miss a delete and leak, easy to have<br>
an early return and fail to take back and release ownership from<br>
something that wasn't really owning in the first place, etc.<br>
<br>
but you had to learn yet another<br>
pointer semantics.<br>
<br>
The marginal gain was that pointer interactions were explicit, making<br>
it easier for someone *not* used to C++ pointer semantics to<br>
understand when reading code, not necessarily when writing it. The<br>
marginal lost was getting people that already knew the STL and Boost<br>
smart pointer semantics to get confused.<br>
<br>
<br>
This is another pointer semantic - I'm not suggesting replacing<br>
unique_ptr or shared_ptr - I want to use them as much as possible where<br>
they represent the right semantic. I'm just dealing with a situation<br>
that doesn't map directly to either of those: conditional ownership.<br>
</blockquote>
<br></span>
I think I may have mentioned null_deleter.<span class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Having done that, I still rather use normal pointers and have valgrind<br>
/ sanitizers tell me when I screwed up.<br>
<br>
My tuppence.<br>
</blockquote>
<br></span><span class="">
consider a leaky example:<br>
{<br>
T* p = new T;<br>
if (condition1) {<br>
f(p); // takes ownership of p<br>
}<br>
p->SomeMethod();<br>
<br>
if (condition2) {<br>
return nullptr; // Leak!<br>
}<br>
<br>
g(p); // don't take ownership of p<br>
return p;<br>
}<br></span></blockquote><div><br></div><div>Yeah, imho this is a somewhat uninteresting example - the issue is just that ownership is passed elsewhere, then after that point a non-owning pointer is required. This is statically known and can be coded statically quite simply:<br><br> unique_ptr ownT = ...;<br> /* stuff */<br> T *t = ownT.get();<br> sink(std::move(ownT));<br> return t;<br><br>I don't think this particular use case really merits designing a new (fairly powerful) abstraction. But it's not necessarily easy to provide a nice example of the issue that's sufficiently motivating.<br><br>The use case I have in mind (of which I've seen several in LLVM and Clang) looks something more like this:<br><br> struct foo {<br> unique_ptr<T> u;<br> foo(unique_ptr<T> u) : u(std::move(u)) {}<br> int stuff();<br> unique_ptr<T> take() { return std::move(u); }<br> };<br><br> int f1(T& t) {<br> // I want to do "stuff" to 't' but I don't own it... <br><br> // lying through my teeth<br> foo f(std::unique_ptr<T>(&t)); <br><br> // I really hope 'stuff' doesn't throw, because then we'll end up with a double delete<br> int i = f.stuff(); </div><div> f.take().release(); // not really leaking because I lied in the first place<br> return i;<br> }<br><br>That's one case - check the revision numbers mentioned in the first message on this thread for another example that doesn't involve a member, just some rather convoluted ownership semantics (some codepaths there's a default object to point at, other codepaths there's a newly allocated object) it looks something like this:<br><br> T *t = nullptr;<br> bool Owning = false;<br> if (x) {<br> t = new T;<br> Owning = true;<br> } else<br> t = &default;<br> func(*t);<br> if (Owning)<br> delete t;<br><br>This comes up not infrequently. The code in func doesn't care if it owns, it just wants to do something. Now obviously in the code I just wrote it could be easily refactored without much duplication:<br><br> if (x)<br> func(*make_unique<T>());<br> else<br> func(default);<br><br>But it's not always that simple. Maybe it can/should be made that simple and we conclude that we don't want/need conditional ownership, but that's what this thread is hopefully here to help decide.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">
<br></span>
Let's see what we can do:<br>
<br>
{<br>
auto p = llvm::make_unique<T>();<span class=""><br>
if (condition1) {<br>
f(p); // takes ownership of p<br>
}<br>
p->SomeMethod();<br>
<br>
if (condition2) {<br></span>
return nullptr; // no leak;<br>
}<br>
<br>
g(p.get()); // don't take ownership of p<br>
return p; // not p.get()!<br>
}<br>
<br>
f(llvm::unique_ptr<T>& p)<br>
{<br>
if(!condition2) // I guess<br>
p = llvm::make_unique<T>(p, null_deleter()); [1]<br>
}<br>
<br>
Clearly the code is a mess as f(llvm::unique_ptr<T>&) needs to know condition2, but smart_ptr as described isn't helping anything.<br>
<br>
[1] std::unique_ptr doesn't have a type erased deleter, but perhaps a unique_ptr with a type erased deleter is actually the right tool for the job.</blockquote><div><br></div><div>Yes, the lack of type erasure is the issue - and then the lack of compatibility with existing unique_ptrs (I'd obviously like to be able to make a conditional ownership unique_ptr from an unconditional ownership unique_ptr&&, for example, and to be able to take unconditional ownership if it is owned, etc)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class=""><font color="#888888"><br>
<br>
Ben</font></span><div class=""><div class="h5"><br>
<br>
______________________________<u></u>_________________<br>
LLVM Developers mailing list<br>
<a href="mailto:LLVMdev@cs.uiuc.edu" target="_blank">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvmdev</a><br>
</div></div></blockquote></div><br></div></div>