[PATCH][tablegen] Eliminate memory leaks in TGParser.cpp

[Sean Silva]

Thanks Anton for checking out these leaks.


On Tue, Aug 5, 2014 at 3:28 PM, David Blaikie <dblaikie at gmail.com> wrote:

> Personally I'd favor:
>
>   auto foo = llvm::make_unique<T>(...);
>   foo = llvm::make_unique<T>(...);
>
> over:
>
>   std::unique_ptr<T> foo(new T(...));
>   foo.reset(new T(...));
>
> Though opinions may vary there.
>

I think this suggestion is quite a bit cleaner.


> I'd probably consider renaming your "CurRecLocalView" to "CurRec" so
> that most of the code doesn't need to change (and CurRecUniquePtr
> maybe "CurRecOwner"?)
>

I found the CurRecLocalView to be confusing. Why is it needed? Adding tons
of explicit .get()'s (and .release()) is a nice way to indicate that this
code still needs more work to get its ownership situation cleaned up.
Hiding that with a local variable just... hides it.

Is it possible to just use a single variable `std::unique_ptr<Record>
CurRec;` and tons of .get()'s and .release()'s? That also ensures a "single
point of truth".

-- Sean Silva


>
> Also in a preliminary/prior patch, it might be worth changing "addDef"
> to take a unique_ptr by value to properly document the ownershp
> passing (I'm hoping to keep focusing on any 'release()' calls and use
> them as a marker of places to clean up/continue pushing unique_ptr
> through).
>
> On Tue, Aug 5, 2014 at 12:20 PM, Anton Yartsev <anton.yartsev at gmail.com>
> wrote:
> > Hi all,
> >
> > I've recently ran the alpha.cplusplus.NewDeleteLeaks checker over the
> LLVM
> > codebase to eliminate false-positives. The checker evolved a number of
> real
> > leaks. I've decided to fix some of them.
> > Attached is the patch that eliminates memory leaks found in the
> > TGParser.cpp. Please review.
> >
> > --
> > Anton
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140805/60ac105a/attachment.html>