[PATCH] Proposal on how to fix temporary dtors.

[Manuel Klimek]

On Mon, Jul 28, 2014 at 3:43 PM, Manuel Klimek <klimek at google.com> wrote:

> Some more context:
> It seems like we already need to do that when building the CFG: for the
> lifetime extended object we must not emit the destructor at the end of the
> full expression, which we currently do.
>

And it looks like we're already trying to do that by handing
BindToTemporary down. I'm investigating some more...


>
>
> On Mon, Jul 28, 2014 at 3:03 PM, Manuel Klimek <klimek at google.com> wrote:
>
>> On Mon, Jul 7, 2014 at 10:15 PM, Jordan Rose <jordan_rose at apple.com>
>> wrote:
>>
>>> Ah...then I'm glad we added the assertion. :-) Lifetime-extended
>>> temporaries aren't quite implemented correctly yet, but we should probably
>>> be removing or not even adding the state when the temporary is
>>> lifetime-extended.
>>>
>>> Unfortunately I don't have this paged in, but there's logic in
>>> processing auto destructors to handle this, and a bug (
>>> http://llvm.org/bugs/show_bug.cgi?id=19539) about how it's wrong.
>>>
>>
>> Ok, I dug into this a bit, and if I'm not missing something I think it's
>> not possible to switch this off without implementing lifetime-extended
>> temporaries correctly (or at least a similarly sized implementation effort):
>> The only link between the MaterializeTempooraryExpr (which has the
>> information whether the lifetime was extended) and the CXXBindTemporaryExpr
>> is the underlying object storage. I looked into CodeGen, and if I
>> understand it correctly, it looks like it basically stores for the
>> destination whether the destructor was already handled (for example from
>> the MaterializeTemporary flow). To get that information, I'd guess we need
>> to do something similar in the static analyzer.
>>
>> I hope that my analysis is wrong and you tell me a better way to fix this
>> :)
>>
>> Thanks!
>> /Manuel
>>
>>
>>>
>>> Jordan
>>>
>>>
>>> On Jul 7, 2014, at 13:13 , Manuel Klimek <klimek at google.com> wrote:
>>>
>>> Sigh. That triggers in Analysis/dtor.cpp for this CXXBindTemporaryExpr:
>>> CXXBindTemporaryExpr 0x3c195d8 'class
>>> LifetimeExtension::SaveOnVirtualDestruct' (CXXTemporary 0x3c195d0)
>>>  `-CXXTemporaryObjectExpr 0x3c19590 'class
>>> LifetimeExtension::SaveOnVirtualDestruct' 'void (void)'
>>>
>>> I assume something fishy is going on for lifetime extended temporaries...
>>>
>>>
>>> On Mon, Jul 7, 2014 at 8:38 PM, Jordan Rose <jordan_rose at apple.com>
>>> wrote:
>>>
>>>>
>>>> On Jul 7, 2014, at 11:37 , Manuel Klimek <klimek at google.com> wrote:
>>>>
>>>> On Mon, Jul 7, 2014 at 8:05 PM, Jordan Rose <jordan_rose at apple.com>
>>>> wrote:
>>>>
>>>>>
>>>>> On Jul 7, 2014, at 10:50 , Manuel Klimek <klimek at google.com> wrote:
>>>>>
>>>>> On Mon, Jul 7, 2014 at 7:42 PM, Jordan Rose <jordan_rose at apple.com>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>> On Jul 7, 2014, at 10:41 , Manuel Klimek <klimek at google.com> wrote:
>>>>>>
>>>>>> On Mon, Jul 7, 2014 at 7:38 PM, Jordan Rose <jordan_rose at apple.com>
>>>>>> wrote:
>>>>>>
>>>>>>>
>>>>>>> On Jul 7, 2014, at 10:37 , Manuel Klimek <klimek at google.com> wrote:
>>>>>>>
>>>>>>> On Mon, Jul 7, 2014 at 7:29 PM, Jordan Rose <jordan_rose at apple.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> On Jul 7, 2014, at 10:28 , Manuel Klimek <klimek at google.com> wrote:
>>>>>>>>
>>>>>>>> On Mon, Jul 7, 2014 at 6:48 PM, Jordan Rose <jordan_rose at apple.com>
>>>>>>>>  wrote:
>>>>>>>>
>>>>>>>>> Can you add an assertion at the end of a block that there are no
>>>>>>>>> outstanding temporary destructors in the current stack frame? That seems
>>>>>>>>> useful.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Do you mean at the end of a VisitBlockDecl?
>>>>>>>>
>>>>>>>>
>>>>>>>> No, during the path-sensitive run, so handleBlockExit.
>>>>>>>>
>>>>>>>
>>>>>>> So you mean at the end of a CFG block? But here we might have
>>>>>>> outstanding temporary dtors open (?)
>>>>>>>
>>>>>>>
>>>>>>> Oops, right. Was thinking too much in terms of AST structure. How
>>>>>>> about at the end of a function (inlined or not)?
>>>>>>>
>>>>>>
>>>>>> Could we say every time we transition from a block with a temp dtor
>>>>>> terminator to a block that does not have a temp dtor terminator (or an
>>>>>> unconditional terminator) we check?
>>>>>>
>>>>>>
>>>>>> That sounds correct, but misses the case where we built the CFG wrong
>>>>>> (forgetting to add the branch in the correct place and thus never getting
>>>>>> to the temp dtor block at all).
>>>>>>
>>>>>
>>>>> Makes sense. Do you have a hint where the right place on function exit
>>>>> to check it would be? :)
>>>>>
>>>>>
>>>>> *checks* ExprEngine::processEndOfFunction.
>>>>>
>>>>
>>>> Hm, so we'll need to adjust the data structure to be indexed by stack
>>>> frame somehow (use a map) instead of the pair<expr, stack-frame>?
>>>>
>>>>
>>>> Eh, since it's an assertion I'd be fine with just iterating over it in
>>>> a helper function. Or better, using std::find_if (if it has proper
>>>> begin/end members).
>>>>
>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140728/c7c1054f/attachment.html>