<div dir="ltr">Ah, yes, that's true. This patch doesn't work indeed.<div><br></div><div>Out of curiosity, using BumpPtrAllocator to allocate atoms is just for speed?</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 10, 2015 at 1:04 PM, Jean-Daniel Dupas <span dir="ltr"><<a href="mailto:mailing@xenonium.com" target="_blank">mailing@xenonium.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">std::vector allocates storage to store the list of plain pointer. Its destructor must be call to free that memory.<div>That changed was driven by memory leaks analysis tool that reports leaks of memory allocated by std::vector, so this is not some just some arbitrary guess.<div><br><div><blockquote type="cite"><div>Le 10 mars 2015 à 20:59, Rui Ueyama <<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>> a écrit :</div><div><div class="h5"><br><div><div dir="ltr">It seems okay to me if _references vector's destructor is not called. It doesn't own references its pointing to -- it contains a list of plain pointers, not unique_ptr or such. So references will be managed somewhere and that's not changed by this patch.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 10, 2015 at 5:17 AM, Jean-Daniel Dupas <span dir="ltr"><<a href="mailto:mailing@xenonium.com" target="_blank">mailing@xenonium.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">My bad, I was thinking about SimpleDefinedAtom allocation and destruction.<br>
Many subclass of SimpleDefinedAtom are allocated using bumpPtrAllocator, (Mach-O, ELF and COFF are all doing this).<br>
In your patch, the vector will be owned by such atoms and its destructor will never be executed.<br>
<br>
> Le 10 mars 2015 à 05:55, Rui Ueyama <<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>> a écrit :<br>
<div><div>><br>
> Where is the code? Looks like SimpleReference is used only by PE/COFF port.<br>
><br>
> On Mon, Mar 9, 2015 at 3:46 PM, Jean-Daniel Dupas <<a href="mailto:mailing@xenonium.com" target="_blank">mailing@xenonium.com</a>> wrote:<br>
> SimpleReference was implemented using a vector not so long ago, but that design choice was made because std::vector is not bumpPtrAllocator friendly.<br>
> Mach-O implementation (at least) uses a bumpPtrAllocator to allocate the SimpleReferences. So SimpleReference destructor is never call, and so are members destructor.<br>
> If you use a std::vector, it means you will leak the vector storage memory. As all ilist nodes are allocated using the bumpPtr allocator, we don’t have that issue.<br>
><br>
><br>
> <a href="http://reviews.llvm.org/D8182" target="_blank">http://reviews.llvm.org/D8182</a><br>
><br>
> EMAIL PREFERENCES<br>
>   <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
><br>
><br>
><br>
</div></div>> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</blockquote></div><br></div>
</div></div></div></blockquote></div><br></div></div></div></blockquote></div><br></div>