[llvm] r208293 - Use a vector of unique_ptrs to fix a memory leak introduced in r208179.

[David Blaikie]

On Thu, May 8, 2014 at 2:29 AM, Daniel Sanders
<daniel.sanders at imgtec.com> wrote:
> Author: dsanders
> Date: Thu May  8 04:29:28 2014
> New Revision: 208293
>
> URL: http://llvm.org/viewvc/llvm-project?rev=208293&view=rev
> Log:
> Use a vector of unique_ptrs to fix a memory leak introduced in r208179.
>
> Also removed an inaccurate comment that stated that a DenseMap was used as
> storage for the ListInit*'s. It's currently using a FoldingSet.

So FoldingSet<T> is really a set of raw, non-owning, T*? Should we
make it FoldingSet<pointer-like-thing> (that would require updating
all existing uses from T to explicitly specify T*) and allow it to
handle unique_ptr<T> correctly as another "pointer-like-thing" so it
can hold ownership?

> I expect there's a better way to fix this but I haven't found it yet. FoldingSet
> is incompatible with the Pool template

Which Pool is that? (can't find a Pool.h... ) Does there seem to be a
fundamental reason FoldingSet+Pool can't play together? If not, should
we just fix that incompatibility?

> and I'm not sure if FoldingSet can be
> safely replaced with a DenseMap of computed FoldingSetID's to ListInit*'s.
>
>
> Modified:
>     llvm/trunk/lib/TableGen/Record.cpp
>
> Modified: llvm/trunk/lib/TableGen/Record.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/TableGen/Record.cpp?rev=208293&r1=208292&r2=208293&view=diff
> ==============================================================================
> --- llvm/trunk/lib/TableGen/Record.cpp (original)
> +++ llvm/trunk/lib/TableGen/Record.cpp Thu May  8 04:29:28 2014
> @@ -623,9 +623,8 @@ static void ProfileListInit(FoldingSetNo
>  ListInit *ListInit::get(ArrayRef<Init *> Range, RecTy *EltTy) {
>    typedef FoldingSet<ListInit> Pool;
>    static Pool ThePool;
> +  static std::vector<std::unique_ptr<ListInit>> TheActualPool;
>
> -  // Just use the FoldingSetNodeID to compute a hash.  Use a DenseMap
> -  // for actual storage.
>    FoldingSetNodeID ID;
>    ProfileListInit(ID, Range, EltTy);
>
> @@ -635,6 +634,7 @@ ListInit *ListInit::get(ArrayRef<Init *>
>
>    ListInit *I = new ListInit(Range, EltTy);
>    ThePool.InsertNode(I, IP);
> +  TheActualPool.push_back(std::unique_ptr<ListInit>(I));

Prefer llvm::make_unique rather than raw new (makes it easier to
ensure ownership is handled everywhere):

auto I = make_unique<ListInit>(Range, EltTy);
ThePool.InsertNode(I.get(), IP);
TheActualPool.push_back(std::move(I));

>    return I;
>  }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits