[cfe-commits] r118991 - in /cfe/trunk: lib/Analysis/CFG.cpp test/Analysis/temp-obj-dtors-cfg-output.cpp

[Marcin Świderski]

W dniu 14 listopada 2010 14:00 użytkownik Zhongxing Xu <
xuzhongxing at gmail.com> napisał:

>
>
> 2010/11/14 Marcin Świderski <marcin.sfider at gmail.com>
>
>> W dniu 14 listopada 2010 03:15 użytkownik Zhongxing Xu <
>> xuzhongxing at gmail.com> napisał:
>>
>>
>>>
>>> 2010/11/14 Marcin Świderski <marcin.sfider at gmail.com>
>>>
>>>> W dniu 14 listopada 2010 00:48 użytkownik Zhongxing Xu <
>>>> xuzhongxing at gmail.com> napisał:
>>>>
>>>>
>>>>>
>>>>> 2010/11/14 Marcin Świderski <marcin.sfider at gmail.com>
>>>>>
>>>>> W dniu 13 listopada 2010 12:37 użytkownik Zhongxing Xu <
>>>>>> xuzhongxing at gmail.com> napisał:
>>>>>>
>>>>>> I see some unbalancing before this patch. (See the test case that was
>>>>>>> modified.) Could you provide an example that is unbalanced by this patch?
>>>>>>>
>>>>>>> 2010/11/13 Marcin Świderski <marcin.sfider at gmail.com>
>>>>>>>
>>>>>>> Hi Zhongxing
>>>>>>>>
>>>>>>>> Won't it lead to unbalancing ctors/dtors like I've written in
>>>>>>>> discusion after commit r118159?
>>>>>>>>
>>>>>>>>
>>>>>>>> When I wrote about unbalancing I meant that removing elidable
>>>>>> CXXConstructExpr and then removing destructors for temporaries that where
>>>>>> created with those expressions will unbalance ctors/dtors calls. Because not
>>>>>> every call to the ctor is a block level statement this is not visualised in
>>>>>> the CFG.
>>>>>>
>>>>>> Example of this is simple:
>>>>>>
>>>>>> class A {
>>>>>> public:
>>>>>>   A() {}
>>>>>>   ~A() {}
>>>>>> };
>>>>>>
>>>>>> A foo() { return A(); }
>>>>>>
>>>>>>  AST for function foo() looks like this:
>>>>>>
>>>>>> A foo() (CompoundStmt 0x105048b98 <main.cpp:7:9, col:23>
>>>>>>   (ReturnStmt 0x105048b70 <col:11, col:20>
>>>>>>     (CXXExprWithTemporaries 0x105048b38 <col:18, col:20> 'class A'
>>>>>>       (CXXTemporary 0x105048a80)
>>>>>>       (CXXConstructExpr 0x105048af0 <col:18, col:20> 'class A''void
>>>>>> (const class A &) throw()' elidable
>>>>>>         (ImplicitCastExpr 0x105048ad0 <col:18, col:20> 'const class A'
>>>>>> <NoOp>
>>>>>>           (CXXBindTemporaryExpr 0x105048a88 <col:18, col:20> 'class A'
>>>>>> (CXXTemporary 0x105048a80)
>>>>>>             (CXXTemporaryObjectExpr 0x105048a38 <col:18, col:20>
>>>>>> 'class A''void (void)')))))))
>>>>>>
>>>>>> If we want to ensure that the CFG is constructed from AST with all
>>>>>> elidable constructors elided, for above example we would have to
>>>>>> ignore both CXXConstructExpr and CXXBindTemporaryExpr. In your
>>>>>> implementation only the first will be elided.
>>>>>>
>>>>>
>>>>> Hi Marcin,
>>>>>
>>>>> I still do not understand your point. The current implementation works
>>>>> well for the above example. One block-level expr for CXXTemporaryObjectExpr.
>>>>> One temporary dtor for CXXBindTemporaryExpr.
>>>>>
>>>>
>>>> If CXXConstructExpr that constructs the return value will be elided,
>>>> object created with CXXTemporaryObjectExpr
>>>> will be the return value and should not be destroyed. So eliding copy
>>>> constructor should also prevent destroying
>>>> temporary from which it would copy, not the other way around.
>>>>
>>>
>>> I see your point. This could be prevented by other means, for example, in
>>> CodeGen there is a LifetimeFlag in AggValueSlot, which controls whether the
>>> temporary would be cleaned up.
>>>
>>> But this reminds me that probably we should not include dtors for
>>> temporaries in the CFG, since not all temporaries require destruction.
>>>
>>> Another option is to create a dtor for every CXXBindTemporaryExpr and let
>>> the client decide whether to ignore it. What do you think?
>>>
>>> Which temporaries, other than those elided, don't require destruction?
>>
>
>  A foo() (CompoundStmt 0x3826c48 <ret.cpp:7:9, col:23>
>   (ReturnStmt 0x3826c20 <col:11, col:20>
>     (CXXExprWithTemporaries 0x3826be8 <col:18, col:20> 'class A'
>       (CXXTemporary 0x3826b30)
>       (CXXConstructExpr 0x3826ba0 <col:18, col:20> 'class A''void (const
> class A &) throw()' elidable
>         (ImplicitCastExpr 0x3826b80 <col:18, col:20> 'const class A' <NoOp>
>           (CXXBindTemporaryExpr 0x3826b38 <col:18, col:20> 'class A'
> (CXXTemporary 0x3826b30)
>             (CXXTemporaryObjectExpr 0x3826ae8 <col:18, col:20> 'class
> A''void (void)')))))))
>
> CXXTemporaryObjectExpr 0x3826ae8 is constructed directly into the return
> value slot, which does not require destruction.
>

That's why I think we shouldn't try to optimize CFG but leave destructors
for temporaries as an option.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20101114/93cdcd98/attachment.html>