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

[Zhongxing Xu]

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?
>
> I think that we should leave destructors for temporaries as an option. Also
> I think we shouldn't optimize elidable constructors and destructors they
> impact in the CFG, because this will force clients to also account for those
> optimizations. This will complicate things without any gain for analysis
> outcome.
>

To clarify, I didn't optimize away elidable constructors. They are just not
appended as block-level expr. They still exist in the CFG.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20101115/13c6a188/attachment.html>