[cfe-dev] [Lifetime, Temporary] Generate ExprWithCleanups for trivially destructible tempoararies

[Richard Smith via cfe-dev]

You should be aware of C++ [basic.life]/1:

"The lifetime of an object of type T ends when:
 — if T is a class type with a non-trivial destructor (12.4), the
destructor call starts, or
 — the storage which the object occupies is reused or released."

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).

So, given:

struct A { ~A(); };
struct B { ~B(); };
void f(int&&);

( A(), f(0), B() );

... should clean up as follows:

1) call ~B()
2) end lifetime of B temporary
3) call ~A()
4) end lifetime of A temporary
5) end lifetime of int temporary

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:

1) call ~B()
2) call ~A()
3-5) end lifetime of temporaries

but not to do this:

1) call ~B()
2) end lifetime of B temporary
3) end lifetime of int temporary
4) call ~A()
5) end lifetime of A temporary

On 28 April 2016 at 11:13, Tim Shen <timshen at google.com> wrote:

> 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.
>
> 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:
>   call void dtor2();
>   call void dtor1();
>   llvm.lifetime.end(t2);
>   llvm.lifetime.end(t1);
>
> instead of, ideally:
>   call void dtor2();
>   llvm.lifetime.end(t2);
>   call void dtor1();
>   llvm.lifetime.end(t1);
>
> This approach is good enough to reduce the stack frame size, though.
>
> On Tue, Apr 26, 2016 at 9:51 PM Tim Shen <timshen at google.com> wrote:
>
>> Hi, I'm working on bug 27451 (https://llvm.org/bugs/show_bug.cgi?id=27451
>> ).
>>
>> If the lifetime-extended temporary object is not trivially destructible,
>> there will be a ExprWithCleanups generated for the VarDecl, so calling
>> CodeGenFunction::pushCleanupAfterFullExpr to push a llvm.lifetime.end()
>> call as a cleanup (as clang currently does in non-temporary cases) just
>> works.
>>
>> However, I learned that ExprWithCleanups will not be generated by Sema
>> ::MaybeBindToTemporary, if the destructor is trivial, according to <
>> https://github.com/llvm-mirror/clang/blob/4a65931dcba82b23856d654eb77d133d0a3c59f2/lib/Sema/SemaExprCXX.cpp#L5593>;
>> and pushCleanupAfterFullExpr seems not working well without a
>> ExprWithCleanups, due to the absence of RunCleanupsScope <
>> https://github.com/llvm-mirror/clang/blob/717e5e20622b0f4ac3c16bb643ed6a36eef29603/lib/CodeGen/CGExpr.cpp#L1002
>> >.
>>
>> If there is a ExprWithCleanups generated even for a trivially
>> destructible temporary, generating llvm.lifetime.end() is easy by calling
>> CodeGenFunction::pushCleanupAfterFullExpr.
>>
>> This requires moving L5593 to L5550 in SemaExprCXX.cpp. 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.
>>
>> Is this - by adding more ExprWithCleanups and fix tests - the correct way to
>> fix the temporary lifetime bug? I feel weird about the existence of
>> ExprWithCleanups in the first place, but that's off the topic.
>>
>> Just for illustration, the attached patch fixes the bug, but doesn't fix
>> the tests though.
>>
>> Thanks!
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160428/0a0fa0bd/attachment.html>