[LLVMdev] New type of smart pointer for LLVM

[David Blaikie]

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).

I'd love to hear some more opinions, but maybe we're not going to get
them...


>
> The main goal of the wrapper is to eliminate leaks like those in TGParser.cpp
> - r215176
> <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/TableGen/TGParser.cpp?r1=215176&r2=215175&pathrev=215176&diff_format=f>.
> With the wrapper the fixes applied at r215176
> <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/TableGen/TGParser.cpp?r1=215176&r2=215175&pathrev=215176&diff_format=f> could
> be refactored in the more easy and safe way.
>
> Attached is a proposed interface/implementation.
> Any ideas, suggestions? Is it OK to move forward with the solution?
>
>  Hello everyone,
>
> I bring to discussion the necessity/design of a new type of smart pointer.
> r215176 and r217791 rise the problem, D5443
> <http://reviews.llvm.org/D5443> is devoted to the solution.
> r215176 applies several temporary ugly fixes of memory leaks in TGParser.cpp
> which would be great to be refactored using smart pointers. D5443
> <http://reviews.llvm.org/D5443> demonstrates how the solution with a
> certain type of smart pointer would look like (see changes in TGParser::ParseDef(),
> TGParser::InstantiateMulticlassDef() and TGParser::ParseSimpleValue()).
>
> Briefly:
> consider a leaky example:
> {
>   T* p = new T;
>   if (condition1) {
>     f(p); // takes ownership of p
>   }
>   p->SomeMethod();
>
>   if (condition2) {
>     return nullptr; // Leak!
>   }
>
>   g(p); // don't take ownership of p
>   return p;
> }
>
> The preferred solution would look like:
> {
>   smart_ptr<T> p(new T);
>   if (condition1) {
>     f(p.StopOwn()); // takes ownership of p
>   }
>   p->SomeMethod();
>
>   if (condition2) {
>     return nullptr; //
>   }
>
>   g(p.Get());  // don't take ownership of p
>   return p.StopOwn();
> }
>
> 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.
>
> 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.
>
> --
> Anton
>
>
>
> --
> Anton
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141001/55f9601f/attachment.html>