thr3ads.net - llvm dev - [LLVMdev] New type of smart pointer for LLVM [Sep 2014]

If this information is useful, please help other people find it:
Share via:

Anton Yartsev

2014-Sep-25 02:12 UTC

[LLVMdev] New type of smart pointer for LLVM

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140925/fb27d622/attachment.html>
-------------- next part --------------
//===-- CondOwnershipPtr.h - Smart ptr with conditional release -*- C++ -*-===//
//
//                     The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//===----------------------------------------------------------------------===//

#ifndef LLVM_ADT_CONDOWNERSHIPPTR_H
#define LLVM_ADT_CONDOWNERSHIPPTR_H

namespace llvm {

template<class T> 
class CondOwnershipPtr {
  T* Ptr;
  bool Own;

  void Delete() {
    if (Ptr && !Own)
      delete Ptr;
  }

public:
  CondOwnershipPtr() : Ptr(nullptr), Own(true) {}
  explicit CondOwnershipPtr(T* p) : Ptr(p), Own(true) {}

  ~CondOwnershipPtr() {
    Delete();
  }

  T* Get() const {
    return Ptr;
  }

  T* StopOwn() const {
    Own = false;
    return Ptr;
  }

  void Reset(T* P = nullptr) {
    if (P != Ptr) {
      Delete();
      Ptr = P;
      Own = true;
    }
  }

  bool Owns() const {
    return Own;
  }

  operator bool() const {
    return Ptr != nullptr;
  }

  T& operator*() const {
    return *Ptr;
  }

  T* operator->() const {
    return Ptr;
  }
};

} // end namespace llvm

#endif

Justin Bogner

2014-Sep-25 02:50 UTC

head link

[LLVMdev] New type of smart pointer for LLVM

Anton Yartsev <anton.yartsev at gmail.com> writes:> Hello everyone,
>
> I bring to discussion the necessity/design of a new type of smart pointer.
> r215176 and r217791 rise the problem, 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
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
So this takes ownership, but promises not to destroy the pointee in some
way?
>   }
>   p->SomeMethod();
>
>   if (condition2) {
>     return nullptr; //
I guess p is sometimes destroyed here, depending on condition1?
>   }
>
>   g(p.Get());  // don't take ownership of p
>   return p.StopOwn();
> }
What does it mean to stop owning the pointer twice? Doesn't this leak p
in the case where condition1 was false?
> 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.
I don't understand why shared_ptr wouldn't suffice for the example
above. There are two cases in your example where you try to release the
pointer - in one of them it seems you don't actually want to release it,
because you continue using it after, and in the other the scope ends
immediately after the release. The shared_ptr releases when it goes out
of scope, so it seems to be exactly what you want here.

Maybe I'm just misunderstanding the use case here, but I fear that a
type like this one would just serve to paper over problems where the
ownership is very unclear, without really helping much.
> 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.
>
> //===-- CondOwnershipPtr.h - Smart ptr with conditional release -*- C++
-*-===//
> //
> //                     The LLVM Compiler Infrastructure
> //
> // This file is distributed under the University of Illinois Open Source
> // License. See LICENSE.TXT for details.
>
//===----------------------------------------------------------------------===//
> 
> #ifndef LLVM_ADT_CONDOWNERSHIPPTR_H
> #define LLVM_ADT_CONDOWNERSHIPPTR_H
> 
> namespace llvm {
> 
> template<class T> 
> class CondOwnershipPtr {
>   T* Ptr;
>   bool Own;
> 
>   void Delete() {
>     if (Ptr && !Own)
>       delete Ptr;
>   }
This seems to delete the pointer iff we *don't* own it. I think you have
that backwards...
> public:
>   CondOwnershipPtr() : Ptr(nullptr), Own(true) {}
>   explicit CondOwnershipPtr(T* p) : Ptr(p), Own(true) {}
> 
>   ~CondOwnershipPtr() {
>     Delete();
>   }
> 
>   T* Get() const {
>     return Ptr;
>   }
> 
>   T* StopOwn() const {
>     Own = false;
>     return Ptr;
>   }
> 
>   void Reset(T* P = nullptr) {
>     if (P != Ptr) {
>       Delete();
>       Ptr = P;
>       Own = true;
>     }
>   }
> 
>   bool Owns() const {
>     return Own;
>   }
> 
>   operator bool() const {
>     return Ptr != nullptr;
>   }
> 
>   T& operator*() const {
>     return *Ptr;
>   }
> 
>   T* operator->() const {
>     return Ptr;
>   }
> };
> 
> } // end namespace llvm
> 
> #endif

David Blaikie

2014-Sep-25 05:16 UTC

head link

[LLVMdev] New type of smart pointer for LLVM

On Wed, Sep 24, 2014 at 7:50 PM, Justin Bogner <mail at justinbogner.com>
wrote:
> Anton Yartsev <anton.yartsev at gmail.com> writes:
> > Hello everyone,
> >
> > I bring to discussion the necessity/design of a new type of smart
> pointer.
> > r215176 and r217791 rise the problem, 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
> 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
>
> So this takes ownership, but promises not to destroy the pointee in some
> way?
>
> >   }
> >   p->SomeMethod();
> >
> >   if (condition2) {
> >     return nullptr; //
>
> I guess p is sometimes destroyed here, depending on condition1?
>
> >   }
> >
> >   g(p.Get());  // don't take ownership of p
> >   return p.StopOwn();
> > }
>
> What does it mean to stop owning the pointer twice? Doesn't this leak p
> in the case where condition1 was false?
>
> > 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.
>
> I don't understand why shared_ptr wouldn't suffice for the example
> above. There are two cases in your example where you try to release the
> pointer - in one of them it seems you don't actually want to release
it,
> because you continue using it after, and in the other the scope ends
> immediately after the release. The shared_ptr releases when it goes out
> of scope, so it seems to be exactly what you want here.
>
> Maybe I'm just misunderstanding the use case here, but I fear that a
> type like this one would just serve to paper over problems where the
> ownership is very unclear, without really helping much.
>
Perhaps this is a case of poor examples, I'm not sure. I certainly share
your concern that this might not be the right path forward, but it's
something that I've seen come up repeatedly when doing unique_ptr
migrations, perhaps moreso in Clang's frontend code.

Essentially there are a variety of data structures that "sometimes"
own
their underlying data. Often implemented as a raw pointer and a boolean.
(sometimes I've migrated this to a raw pointer and a unique_ptr - the raw
pointer always points to the thing of interest, the unique_ptr is sometimes
non-null and points to the same thing to keep it alive (or destroy it when
it's no longer needed, more specifically))

I can go & dredge up some examples if we want to discuss the particular
merits & whether each of those cases would be better solved in some other
way, but it seemed pervasive enough in the current codebase that some
incremental improvement could be gained by replacing these cases with a
more specific tool for the job. We might still consider this tool to be
"not ideal".

>
> > 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.
> >
> > //===-- CondOwnershipPtr.h - Smart ptr with conditional release -*-
C++
> -*-===//
> > //
> > //                     The LLVM Compiler Infrastructure
> > //
> > // This file is distributed under the University of Illinois Open
Source
> > // License. See LICENSE.TXT for details.
> >
>
//===----------------------------------------------------------------------===//
> >
> > #ifndef LLVM_ADT_CONDOWNERSHIPPTR_H
> > #define LLVM_ADT_CONDOWNERSHIPPTR_H
> >
> > namespace llvm {
> >
> > template<class T>
> > class CondOwnershipPtr {
> >   T* Ptr;
> >   bool Own;
> >
> >   void Delete() {
> >     if (Ptr && !Own)
> >       delete Ptr;
> >   }
>
> This seems to delete the pointer iff we *don't* own it. I think you
have
> that backwards...
>
> > public:
> >   CondOwnershipPtr() : Ptr(nullptr), Own(true) {}
> >   explicit CondOwnershipPtr(T* p) : Ptr(p), Own(true) {}
> >
> >   ~CondOwnershipPtr() {
> >     Delete();
> >   }
> >
> >   T* Get() const {
> >     return Ptr;
> >   }
> >
> >   T* StopOwn() const {
> >     Own = false;
> >     return Ptr;
> >   }
> >
> >   void Reset(T* P = nullptr) {
> >     if (P != Ptr) {
> >       Delete();
> >       Ptr = P;
> >       Own = true;
> >     }
> >   }
> >
> >   bool Owns() const {
> >     return Own;
> >   }
> >
> >   operator bool() const {
> >     return Ptr != nullptr;
> >   }
> >
> >   T& operator*() const {
> >     return *Ptr;
> >   }
> >
> >   T* operator->() const {
> >     return Ptr;
> >   }
> > };
> >
> > } // end namespace llvm
> >
> > #endif
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140924/4cbbce90/attachment.html>

Anton Yartsev

2014-Sep-25 14:55 UTC

head link

[LLVMdev] New type of smart pointer for LLVM

On 25.09.2014 6:50, Justin Bogner wrote:> Anton Yartsev <anton.yartsev at gmail.com> writes:
>> Hello everyone,
>>
>> I bring to discussion the necessity/design of a new type of smart
pointer.
>> r215176 and r217791 rise the problem, 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
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
> So this takes ownership, but promises not to destroy the pointee in some
> way?
Yes.>>    }
>>    p->SomeMethod();
>>
>>    if (condition2) {
>>      return nullptr; //
> I guess p is sometimes destroyed here, depending on condition1?
Yes.>
>>    }
>>
>>    g(p.Get());  // don't take ownership of p
>>    return p.StopOwn();
>> }
> What does it mean to stop owning the pointer twice? Doesn't this leak p
> in the case where condition1 was false?The above example just illustrate the typical common patterns. By the 
way, this particular example with several insignificant replaces is much 
similar to the logic of TGParser::InstantiateMulticlassDef() (r215176 
<http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/TableGen/TGParser.cpp?r1=215176&r2=215175&pathrev=215176&diff_format=f>,
memory is allocated at line 2406, at line 2457 Records.addDef(CurRec) 
takes ownership of CurRec). More examples starting from line 2020, 
method TGParser::ParseDef().
>> 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.
> I don't understand why shared_ptr wouldn't suffice for the example
> above. There are two cases in your example where you try to release the
> pointer - in one of them it seems you don't actually want to release
it,
> because you continue using it after, and in the other the scope ends
> immediately after the release. The shared_ptr releases when it goes out
> of scope, so it seems to be exactly what you want here.
>
> Maybe I'm just misunderstanding the use case here, but I fear that a
> type like this one would just serve to paper over problems where the
> ownership is very unclear, without really helping much.I agree the solution is not ideal, but it looks much safer to me then 
just adding and switching extra boolean variables indicating whether the 
memory should be freed or not and inserting missing, sometimes 
conditional, 'delete's. With the suggested solution you should only 
notice and call StopOwn() in places, where ownership is transferred.
The ownership logic is quite simple: "the wrapped memory should be freed 
until otherwise stated". To be more clear I also intend to design the 
wrapper for local scope usage only preventing it from copiying/moving.
>> 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.
>>
>> //===-- CondOwnershipPtr.h - Smart ptr with conditional release -*- C++
-*-===//
>> //
>> //                     The LLVM Compiler Infrastructure
>> //
>> // This file is distributed under the University of Illinois Open
Source
>> // License. See LICENSE.TXT for details.
>>
//===----------------------------------------------------------------------===//
>>
>> #ifndef LLVM_ADT_CONDOWNERSHIPPTR_H
>> #define LLVM_ADT_CONDOWNERSHIPPTR_H
>>
>> namespace llvm {
>>
>> template<class T>
>> class CondOwnershipPtr {
>>    T* Ptr;
>>    bool Own;
>>
>>    void Delete() {
>>      if (Ptr && !Own)
>>        delete Ptr;
>>    }
> This seems to delete the pointer iff we *don't* own it. I think you
have
> that backwards...Right, thanks!
>
>> public:
>>    CondOwnershipPtr() : Ptr(nullptr), Own(true) {}
>>    explicit CondOwnershipPtr(T* p) : Ptr(p), Own(true) {}
>>
>>    ~CondOwnershipPtr() {
>>      Delete();
>>    }
>>
>>    T* Get() const {
>>      return Ptr;
>>    }
>>
>>    T* StopOwn() const {
>>      Own = false;
>>      return Ptr;
>>    }
>>
>>    void Reset(T* P = nullptr) {
>>      if (P != Ptr) {
>>        Delete();
>>        Ptr = P;
>>        Own = true;
>>      }
>>    }
>>
>>    bool Owns() const {
>>      return Own;
>>    }
>>
>>    operator bool() const {
>>      return Ptr != nullptr;
>>    }
>>
>>    T& operator*() const {
>>      return *Ptr;
>>    }
>>
>>    T* operator->() const {
>>      return Ptr;
>>    }
>> };
>>
>> } // end namespace llvm
>>
>> #endif
-- 
Anton

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140925/87884c2e/attachment.html>

Anton Yartsev

2014-Oct-01 22:14 UTC

head link

[LLVMdev] New type of smart pointer for LLVM

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.

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/20141002/a4a5bd1c/attachment.html>
-------------- next part --------------
//===-- CondOwnershipPtr.h - Smart ptr with conditional release -*- C++ -*-===//
//
//                     The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//===----------------------------------------------------------------------===//

#ifndef LLVM_ADT_CONDOWNERSHIPPTR_H
#define LLVM_ADT_CONDOWNERSHIPPTR_H

#include <memory>

namespace llvm {

template<class T> 
class CondOwnershipPtr {
  T* Ptr;
  bool Own;

  void Delete() {
    if (Ptr && Own)
      delete Ptr;
  }

  CondOwnershipPtr(const CondOwnershipPtr&); // Prevent from misuse.
  CondOwnershipPtr& operator=(const CondOwnershipPtr&); // Prevent from
misuse.

public:
  CondOwnershipPtr() : Ptr(nullptr), Own(false) {}
  explicit CondOwnershipPtr(T* p) : Ptr(p) { Own = Ptr && true; }

  ~CondOwnershipPtr() {
    Delete();
  }

  T* Get() const {
    return Ptr;
  }

  T* StopOwn() const {
    Own = false;
    return Ptr;
  }

  typename std::unique_ptr<T> MakeUnique() {
    assert((!Ptr || Own) && "Ownership was transferred.");
    return std::unique_ptr<T>(StopOwn());
  }

  void Reset(T* P = nullptr) {
    if (P != Ptr) {
      Delete();
      Ptr = P;
      Own = Ptr && true;
    }
  }

  bool Owns() const {
    return Ptr && Own;
  }

  operator bool() const {
    return Ptr != nullptr;
  }

  T& operator*() const {
    return *Ptr;
  }

  T* operator->() const {
    return Ptr;
  }
};

} // end namespace llvm

#endif

David Blaikie

2014-Oct-01 22:36 UTC

head link

[LLVMdev] New type of smart pointer for LLVM

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>

Seemingly Similar Threads

Search for more seemingly similar threads

llvm dev - Sep 2014 - [LLVMdev] New type of smart pointer for LLVM

[LLVMdev] New type of smart pointer for LLVM

[LLVMdev] New type of smart pointer for LLVM

[LLVMdev] New type of smart pointer for LLVM

[LLVMdev] New type of smart pointer for LLVM

[LLVMdev] New type of smart pointer for LLVM

[LLVMdev] New type of smart pointer for LLVM

Seemingly Similar Threads