[PATCH] D69674: [FIX] Make LSan happy by *not* leaking memory

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D69674#1753895 <https://reviews.llvm.org/D69674#1753895>, @dblaikie wrote:

> In D69674#1753862 <https://reviews.llvm.org/D69674#1753862>, @jdoerfert wrote:
>
> > In D69674#1752604 <https://reviews.llvm.org/D69674#1752604>, @dblaikie wrote:
> >
> > > Looks like this was sent for review but no one reviewed/approved it? Generally, if something is sent for review, it's not great to submit it without review. (if you felt it needed review to begin with, time shouldn't change that)
> >
> >
> > The review was intended to share the patch and validate the fix, that is why I didn't add reviewers. I will not open a review next time.
>
>
> Validate in what sense? There didn't appear to be any discussion/validation from other developers here.


As far as I can recall, I was pinged on IRC and made aware of the leak, the patch was put here, run on their end to verify, then pushed and closed.

>>> Any chance of using unique_ptrs here, instead of raw pointers and DeleteContainerPointers?
>> 
>> I can do that if you want me to. FWIW, it is a printer pass so I don't feel a strong need to improve performance here and I doubt there is "active development" on this part.
> 
> Ah, it wasn't so much about performance (I don't expect the unique_ptr solution to be any faster) but readability/maintainability - unique_ptr makes it harder to reintroduce bugs if someone adds an early return to that function or otherwise refactors it.

I do not disagree. Will try to make it happen once I find the time.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69674/new/

https://reviews.llvm.org/D69674