<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Sep 10, 2014 at 9:28 AM, Frédéric Riss <span dir="ltr"><<a href="mailto:friss@apple.com" target="_blank">friss@apple.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"><br><div><div><div class="h5"><blockquote type="cite"><div>On 10 Sep 2014, at 18:21, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><br><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div class="gmail_quote" style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">On Wed, Sep 10, 2014 at 9:03 AM, Frederic Riss<span> </span><span dir="ltr"><<a href="mailto:friss@apple.com" target="_blank">friss@apple.com</a>></span><span> </span>wrote:<br><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: friss<br>Date: Wed Sep 10 11:03:14 2014<br>New Revision: 217514<br><br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project?rev=217514&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=217514&view=rev</a><br>Log:<br>Fix comments of createReplaceableForwardDecl() and createForwardDecl().<br><br>Noticed while trying to understand how the merge of forward decalred types<br>and defintions work.<br><br>Reviewers: echristo, dblaikie, aprantl<br><br>Subscribers: llvm-commits<br><br>Differential Revision:<span> </span><a href="http://reviews.llvm.org/D5291" target="_blank">http://reviews.llvm.org/D5291</a><br><br>Modified:<br>   <span> </span>llvm/trunk/lib/IR/DIBuilder.cpp<br><br>Modified: llvm/trunk/lib/IR/DIBuilder.cpp<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=217514&r1=217513&r2=217514&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=217514&r1=217513&r2=217514&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/IR/DIBuilder.cpp (original)<br>+++ llvm/trunk/lib/IR/DIBuilder.cpp Wed Sep 10 11:03:14 2014<br>@@ -879,8 +879,7 @@ DIBasicType DIBuilder::createUnspecified<br>   return DIBasicType();<br> }<br><br>-/// createForwardDecl - Create a temporary forward-declared type that<br>-/// can be RAUW'd if the full type is seen.<br>+/// createForwardDecl - Create a permanent forward-declared type.<br> DICompositeType<br> DIBuilder::createForwardDecl(unsigned Tag, StringRef Name, DIDescriptor Scope,<br>                             <span> </span>DIFile F, unsigned Line, unsigned RuntimeLang,<br>@@ -914,7 +913,7 @@ DIBuilder::createForwardDecl(unsigned Ta<br>   return RetTy;<br> }<br><br>-/// createForwardDecl - Create a temporary forward-declared type that<br>+/// createReplaceableForwardDecl - Create a temporary forward-declared type that<br> /// can be RAUW'd if the full type is seen.<br></blockquote><div><br></div><div>FWIW (either just for your information, or something you can enshrine in a comment): Because this creates a temporary (perhaps we should replace "replaceable" with "temporary") node, it /must/ be RAUW'd at some point, sooner or later. It isn't optional.</div><div> </div></div></div></blockquote><div><br></div></div></div><div>Interesting. I was just working on another patch that adds this kind of temporaries for function and variable forward definitions (for the missed imported decl and my default argument attributes). The comment of MDNode::getTemporary() reads:</div><div><br></div><div><div>  /// getTemporary - Return a temporary MDNode, for use in constructing</div><div>  /// cyclic MDNode structures. A temporary MDNode is not uniqued,</div><div>  /// may be RAUW'd, and must be manually deleted with deleteTemporary.</div><div><br></div><div>I had read the ‘must be manually deletes’ part as ‘if you want to get rid of if, please call deleteTemporary’ but you’re saying that these nodes must not survive the code generation or something will go wrong?</div></div></div></div></blockquote><div><br>Thanks for quoting the docs - I had forgotten the specifics (you might want to look at the commit history around here - I had to make changes to these forward decls to fix memory leaks, reading up on that may help you avoid similar memory leaks as you implement similar functionality).<br><br>The crux is the thing must be manually deleted with deleteTemporary at some point... I don't know if that means it can survive codegen or not - if you had a hook to delete it later, maybe that would be OK (I mean it is OK - ultimately when we used to leak these forward decl nodes, we still got the right metadata out - so I guess if we could hook into it later and delete it after we finish using the IR), but in reality I think that means replacing the temporary nodes, explicitly deleting them (deleteTemporary) and moving on.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><div><br></div><div>Fred</div><div><br></div></div><span class=""><br><blockquote type="cite"><div><div class="gmail_quote" style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing: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"> DICompositeType DIBuilder::createReplaceableForwardDecl(<br>     unsigned Tag, StringRef Name, DIDescriptor Scope, DIFile F, unsigned Line,<br>@@ -942,7 +941,7 @@ DICompositeType DIBuilder::createReplace<br>   MDNode *Node = MDNode::getTemporary(VMContext, Elts);<br>   DICompositeType RetTy(Node);<br>   assert(RetTy.isCompositeType() &&<br>-         "createForwardDecl result should be a DIType");<br>+         "createReplaceableForwardDecl result should be a DIType");<br>   if (!UniqueIdentifier.empty())<br>     retainType(RetTy);<br>   return RetTy;<br><br><br>_______________________________________________<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></blockquote></div></div></blockquote></span></div><br></div></blockquote></div><br></div></div>