<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Mar 21, 2016, at 9:11 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><br class="Apple-interchange-newline"><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On Mon, Mar 21, 2016 at 9:08 PM, Pete Cooper<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:peter_cooper@apple.com" target="_blank" class="">peter_cooper@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;" class=""><br class=""><div class=""><blockquote type="cite" class=""><span class=""><div class="">On Mar 21, 2016, at 9:05 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank" class="">dblaikie@gmail.com</a>> wrote:</div><br class=""></span><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote"><span class="">On Mon, Mar 21, 2016 at 8:44 PM, Pete Cooper via llvm-commits<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">Author: pete<br class="">Date: Mon Mar 21 22:44:32 2016<br class="">New Revision: 264022<br class=""><br class="">URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=264022&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=264022&view=rev</a><br class="">Log:<br class="">Use owning pointers instead of raw pointers for Atom's to fix leaks.<br class=""><br class="">Currently each File contains an BumpPtrAllocator in which Atom's are<br class="">allocated. Some Atom's contain data structures like std::vector which<br class="">leak as we don't run ~Atom when they are BumpPtrAllocate'd.<br class=""></blockquote><div class=""><br class=""></div></span><div class="">FWIW, if the only thing allocated in the BumpPtrAllocator is Atoms, you could use a SpecificBumpPtrAllocator, which does run the doors.<br class=""></div></div></div></div></div></blockquote>I think there might be Reference’s in there, but worth investigating doing it this way. Unfortunately the Window’s bots didn’t like what I did here so i’ve reverted for now.<span class=""><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""><br class="">(& if the BumpPtrAllocator doesn't contain only Atoms, you could change it so it does)</div><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><br class="">Now each File actually owns its Atom's using an OwningAtomPtr. This<br class="">is analygous to std::unique_ptr and may be replaced by it if possible.<br class=""></blockquote><div class=""><br class="">Yeah, this looks like it could just be a typedef of unique_ptr with a custom deleter that only runs the dtor but doesn't delete, etc.</div></div></div></div></div></blockquote></span>Just tried this as a quick fix to get the bots to work. Unfortunately the YAML parser doesn’t like that solution. The problem is that the YAML parser wants to take a reference to a pointer, so i had to make the get() methods be defined as follows to support that. std::unique_ptr doesn’t return a reference from get() so the YAML parser stops working with them.</div></div></blockquote><div class=""><br class=""></div><div class="">Ah, yeah, that's not a safe operation (in the mutable case, especially) for a smart pointer - so if we're going to smart pointerify these things, probably need to adjust that code no matter which smart pointer we use.<br class=""></div></div></div></blockquote>Yeah, Lang and I just went over that code. We probably need to rewrite the YAML code, possibly all the YAML code, to not require the references. I’ve recommitted the code as it was for now to get us something in that doesn’t leak all over the place. Then we can refactor it with the bots backing us up when we do it wrong.<br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class=""><br class="">Did you try making the dtor virtual to fix MSVC's issues? If you are destroying derived objects your dtor does need to be virtual if you're calling it from a base object expression. </div></div></div></blockquote>Turns out the dtor was virtual, but that MSVC didn’t like calling the base class dtor on a derived class. So given ‘DefinedAtom *a’, 'a->~Atom()’ generated an error. I’ve fixed that in the recommit to see if that appeases the bots.</div><div><br class=""></div><div>Cheers,</div><div>Pete<br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;" class=""><div class=""><span class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><br class="">+ T *const &get() const {<br class="">+ return atom;<br class="">+ }<br class="">+<br class="">+ T *&get() {<br class="">+ return atom;<br class="">+ }<br class="">+</blockquote><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><br class=""></blockquote></div></div></div></div></blockquote><div class=""><br class=""></div></span><div class="">Cheers,</div><div class="">Pete</div></div></div></blockquote></div></div></blockquote></div><br class=""></body></html>