[LLVMdev] New type of smart pointer for LLVM (original) (raw)
David Blaikie dblaikie at gmail.com
Wed Oct 1 15:36:08 PDT 2014
- Previous message: [LLVMdev] New type of smart pointer for LLVM
- Next message: [LLVMdev] New type of smart pointer for LLVM
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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 uniqueptr 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&diffformat=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&diffformat=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: { smartptr 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 uniqueptr nor sharedptr can be used in the place of smartptr as uniqueptr sets the raw pointer to nullptr after release() (StopOwn() in the example above) whereas sharedptr 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>
- Previous message: [LLVMdev] New type of smart pointer for LLVM
- Next message: [LLVMdev] New type of smart pointer for LLVM
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]