<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"><div class="im">On Jun 6, 2013, at 9:32 AM, Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:<br>

> Itanium destroys them in the caller at the end of the full expression,<br>
<br>
</div>Um.  This is standard-dictated behavior.  If the MS ABI really gets this<br>
wrong, that is a very serious divergence.<br></blockquote><div><br></div><div style>Yep.</div><div> </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">

> but MSVC destroys them in the callee..<br>
<div class="im"><br>
<br>
> This should help clang compile MSVC's debug iterators more correctly.<br>
> There is still an outstanding issue of a memcpy emitted by the LLVM<br>
> backend, which is not correct for C++ records.<br>
<br>
</div>+  // In the Microsoft C++ ABI, all aggregate arguments are destructed by the<br>
+  // callee, no matter how they are passed at the machine level.<br>
+  bool calleeDestructed =<br>
+    getTarget().getCXXABI().isTemporaryDestroyedByCallee();<br>
+<br>
   if (E->isGLValue()) {<br>
     assert(E->getObjectKind() == OK_Ordinary);<br>
-    return args.add(EmitReferenceBindingToExpr(E, /*InitializedDecl=*/0),<br>
-                    type);<br>
+    return args.add(EmitReferenceBindingToExpr(E, /*InitializedDecl=*/0,<br>
+                                               calleeDestructed), type);<br>
<br>
This is not correct.  There is no way that the Microsoft ABI causes the<br>
callee to destroy arbitrary objects that were passed to it by reference.<br></blockquote><div><br></div><div style>This is just poorly written.  I have tests to ensure that references aren't destroyed by the callee.  This bool only kicks in if the argument has an aggregate evaluation kind, which references don't have.</div>
<div> </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">
--- 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><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">

-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><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">

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><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">
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>