<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 1, 2014 at 3:14 PM, Anton Yartsev <span dir="ltr"><<a 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">
<div text="#000000" bgcolor="#FFFFFF">
<div>Ping!<span><br>
</span><br>
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. <br>
The main difference from unique_ptr is an ability to access the
wrapped pointer after the ownership is transferred.<br>
To make the ownership clearer the wrapper is designed for
local-scope usage only.<br></div></div></blockquote><div><br></div><div>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).<br><br>I'd love to hear some more opinions, but maybe we're not going to get them... </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF"><div>
<br>
The main goal of the wrapper is to eliminate leaks like those in <span>TGParser.cpp -</span><span><span> </span><span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/TableGen/TGParser.cpp?r1=215176&r2=215175&pathrev=215176&diff_format=f" target="_blank">r215176</a></span>.
With the wrapper the fixes </span><span>applied at </span><span><span><span> </span><span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/TableGen/TGParser.cpp?r1=215176&r2=215175&pathrev=215176&diff_format=f" target="_blank">r215176</a>
</span></span>could be refactored in the more easy and safe
way.</span><br>
<br>
Attached is a proposed interface/implementation.<br>
Any ideas, suggestions? Is it OK to move forward with the
solution?<br>
<br>
</div><div><div class="h5">
<blockquote type="cite">
Hello everyone,<br>
<br>
I bring to discussion the necessity/design of a new type of smart
pointer. r215176 and r217791 rise the problem, <a href="http://reviews.llvm.org/D5443" target="_blank">D5443</a>
is devoted to the solution.<br>
r215176 applies several temporary ugly fixes of memory leaks in <span>TGParser.cpp </span>which would be great to be
refactored using smart pointers. <a href="http://reviews.llvm.org/D5443" target="_blank">D5443</a> demonstrates how
the solution with a certain type of smart pointer would look like
(see changes in <span>TGParser::ParseDef(),
TGParser::InstantiateMulticlassDef() and
TGParser::ParseSimpleValue()</span>).<br>
<br>
Briefly:<br>
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>
<br>
The preferred solution would look like:<br>
{<br>
smart_ptr<T> p(new T);<br>
if (condition1) {<br>
f(p.StopOwn()); // takes ownership of p<br>
}<br>
p->SomeMethod();<br>
<br>
if (condition2) {<br>
return nullptr; // <br>
}<br>
<br>
g(p.Get()); // don't take ownership of p<br>
return p.StopOwn();<br>
}<br>
<br>
Neither unique_ptr nor shared_ptr can be used in the place of
smart_ptr as unique_ptr sets the raw pointer to nullptr after
release() (StopOwn() in the example above) whereas shared_ptr is
unable to release.<br>
<br>
Attached is a scratch that illustrates how the minimal
API/implementation of a desired smart pointer sufficient for
refactoring would look like. Any ideas and suggestions are
appreciated.<br>
<br>
<pre cols="72">--
Anton</pre>
</blockquote>
<br>
<br>
</div></div><span class="HOEnZb"><font color="#888888"><pre cols="72">--
Anton</pre>
</font></span></div>
</blockquote></div><br></div></div>