<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Jun 6, 2013, at 1:28 PM, Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:</div><blockquote type="cite"><div dir="ltr">On Thu, Jun 6, 2013 at 4:06 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<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; position: static; z-index: auto; "><div class="im">On Jun 6, 2013, at 9:32 AM, Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:</div></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; position: static; z-index: auto; ">
--- lib/CodeGen/CGDecl.cpp<br>
+++ lib/CodeGen/CGDecl.cpp<br>
@@ -1621,6 +1621,14 @@<br>
   if (!Ty->isConstantSizeType() ||<br>
       !CodeGenFunction::hasScalarEvaluationKind(Ty)) {<br>
     DeclPtr = Arg;<br>
+    // Push a destructor cleanup for this parameter if the ABI requires it.<br>
+    if (!hasScalarEvaluationKind(Ty) &&<br>
<br>
You are immediately dominated by this check already.<br></blockquote><div><br></div><div style="">It's an ||, not an &&?</div></div></div></div></blockquote><div><br></div>Ah, yes, you are right.  So you should cache this result;  it's not exactly</div><div>*expensive* to compute, but it's not free either.</div><div> </div><div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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; position: static; z-index: auto; ">

-RValue CodeGenFunction::EmitAnyExprToTemp(const Expr *E) {<br>
+RValue CodeGenFunction::EmitAnyExprToTemp(const Expr *E, bool ExternallyDestructed) {<br>
<br>
There's already an IsDestructed_t;  please use it.<br></blockquote><div><br></div><div style="">OK.  It seemed like an internal detail of AggValueSlot.</div></div></div></div></blockquote><div><br></div><div>It's a type-safe boolean idiom.</div><br><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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; position: static; z-index: auto; ">

These tricks with externally-destructed aren't really suffiicient;  you need to<br>
push an EH-only cleanup for the object and then disable it when the call<br>
occurs.<br></blockquote><div><br></div><div style="">So, something like, take a stable iterator to the cleanup stack, emit the args, then delete any unnecessary cleanups?  I can't think of an easy way to identify which cleanups were passed to args and which were not.</div></div></div></div></blockquote><div><br></div>(I did see your response.)</div><div><br></div><div>The idea is that the CallArgList would collect a list of cleanups to</div><div>deactivate (in reverse order by preference) before making the call.</div><div> </div><div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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; position: static; z-index: auto; ">
The appropriate solution here is not to propagate IsExternallyDestructed<br>
through umpteen layers of API; you'll need to check for this case specifically<br>
when emitting an argument and do all the appropriate management.</blockquote><div><br></div><div style="">I am doing the check in CodeGenFunction::EmitCallArg().  Maybe I should be propagating a bool InArgumentContext or something instead?</div>
</div></div></div>
</blockquote><br></div><div>You need to inline a specialized path into EmitCallArg, much like several</div><div>of the other specialized paths there, so that you can handle the cleanup</div><div>semantics specially.</div><div><br></div><div>John.</div></body></html>