<div dir="ltr">You should be aware of C++ [basic.life]/1:<div><br></div><div>"The lifetime of an object of type T ends when:<div> — if T is a class type with a non-trivial destructor (12.4), the destructor call starts, or<br></div><div> — the storage which the object occupies is reused or released."<br></div><div><br></div><div>That is, for temporaries that have a trivial destructor or temporaries of a non-class type, the lifetime extends until the storage goes away (after *all* relevant destructors have run).</div><div><br></div><div>So, given:</div><div><br></div><div>struct A { ~A(); };</div><div><div>struct B { ~B(); };</div></div><div>void f(int&&);</div><div><br></div><div>( A(), f(0), B() );</div><div><br></div><div>... should clean up as follows:</div><div><br></div><div>1) call ~B()</div><div>2) end lifetime of B temporary</div><div>3) call ~A()</div><div>4) end lifetime of A temporary</div><div>5) end lifetime of int temporary</div><div><br></div><div>That's obviously stupid, but that's what the language rules require today. (I'm trying to get this fixed but have not succeeded yet.) It's valid for us to do this:</div><div><br></div><div>1) call ~B()</div><div>2) call ~A()</div><div>3-5) end lifetime of temporaries</div><div><br></div><div>but not to do this:</div><div class="gmail_extra"><br></div><div class="gmail_extra"><div>1) call ~B()</div><div>2) end lifetime of B temporary</div><div>3) end lifetime of int temporary</div><div>4) call ~A()</div><div>5) end lifetime of A temporary</div><div><br></div><div class="gmail_quote">On 28 April 2016 at 11:13, Tim Shen <span dir="ltr"><<a href="mailto:timshen@google.com">timshen@google.com</a>></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"><div dir="ltr">As discussed off the list, ExprWithCleanups should be used for dtors only. So another solution can be creating a new LifetimeEndStack in CodeGenFunction, which is handled separately.<div><br></div><div>The problem of this solution is that, since LifetimeEnd marks and dtor calls are not in the same stack, they are not emitted interleaved. The result will be like:</div><div> call void dtor2();</div><div> call void dtor1();</div><div> llvm.lifetime.end(t2);</div><div> llvm.lifetime.end(t1);</div><div><br></div><div>instead of, ideally:</div><div><div> call void dtor2();</div><div><span style="line-height:1.5"> llvm.lifetime.end(t2);</span><br></div><div><div> call void dtor1();</div></div><div> llvm.lifetime.end(t1);</div></div><div><br></div><div>This approach is good enough to reduce the stack frame size, though.</div><div><br><div class="gmail_quote"><div dir="ltr">On Tue, Apr 26, 2016 at 9:51 PM Tim Shen <<a href="mailto:timshen@google.com">timshen@google.com</a>> wrote:<br></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"><div dir="ltr">Hi, I'm working on bug 27451 (<a href="https://llvm.org/bugs/show_bug.cgi?id=27451">https://llvm.org/bugs/show_bug.cgi?id=27451</a>).<div><div><br></div><div><div>If the lifetime-extended temporary object is not trivially destructible, there will be a ExprWithCleanups generated for the VarDecl, so calling <span style="line-height:1.5">CodeGenFunction::</span><span style="line-height:1.5">pushCleanupAfterFullExpr to push a llvm.lifetime.end() call as a cleanup (as clang currently does in non-temporary cases) just works.</span></div><div><br></div></div><div><span style="line-height:1.5">However, I learned that ExprWithCleanups will not be generated by </span><span style="line-height:1.5">Sema</span><span style="line-height:1.5">::MaybeBindToTemp</span><span style="line-height:1.5">orary, if the destructor is trivial, according to </span><span style="line-height:1.5"><</span><a href="https://github.com/llvm-mirror/clang/blob/4a65931dcba82b23856d654eb77d133d0a3c59f2/lib/Sema/SemaExprCXX.cpp#L5593" style="line-height:1.5">https://github.com/llvm-mirror/clang/blob/4a65931dcba82b23856d654eb77d133d0a3c59f2/lib/Sema/SemaExprCXX.cpp#L5593</a><span style="line-height:1.5">>; and pushCleanupAfterFullExpr seems not working well without a ExprWithCleanups, due to the absence of </span><span style="line-height:1.5">RunCleanupsScope <</span><a href="https://github.com/llvm-mirror/clang/blob/717e5e20622b0f4ac3c16bb643ed6a36eef29603/lib/CodeGen/CGExpr.cpp#L1002">https://github.com/llvm-mirror/clang/blob/717e5e20622b0f4ac3c16bb643ed6a36eef29603/lib/CodeGen/CGExpr.cpp#L1002</a>><span style="line-height:1.5">.</span></div>
</div><div><div><br></div><div><span style="line-height:1.5">If there is a ExprWithCleanups generated even for a trivially destructible temporary, generating llvm.lifetime.end() is easy by calling CodeGenFunction::pushCleanupAfterFullExpr.</span></div><div><span style="line-height:1.5"><br></span></div><div><span style="line-height:1.5">This requires moving L5593 to L5550 in SemaExprCXX.cpp. </span><span style="line-height:1.5">The two-lines change breaks 36 tests, most of which are checking for warnings, so I suppose I also need to fix some warning detections.</span></div><div><span style="line-height:1.5"><br></span></div><div><span style="line-height:1.5">Is this - by adding more ExprWithCleanups and fix tests -</span><span style="line-height:1.5"> </span><span style="line-height:1.5">the correct way</span><span style="line-height:1.5"> to fix the temporary lifetime bug? I feel weird about the existence of ExprWithCleanups in the first place, but that's off the topic.</span></div></div><div><span style="line-height:1.5"><br></span></div><div><span style="line-height:1.5">Just for illustration, the attached patch fixes the bug, but doesn't fix the tests though.</span></div><div><span style="line-height:1.5"><br></span></div><div><span style="line-height:1.5">Thanks!</span></div></div></blockquote></div></div></div>
</blockquote></div><br></div></div></div>