[cfe-commits] r90283 - in /cfe/trunk: lib/Sema/Sema.h lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExprCXX.cpp test/CodeGenCXX/virtual-destructor-synthesis.cpp

[Fariborz Jahanian]

On Dec 1, 2009, at 7:33 PM, Fariborz Jahanian wrote:

>
> On Dec 1, 2009, at 6:25 PM, Eli Friedman wrote:
>
>> On Tue, Dec 1, 2009 at 6:00 PM, Fariborz Jahanian
>> <fjahanian at apple.com> wrote:
>>>
>>> On Dec 1, 2009, at 3:43 PM, Eli Friedman wrote:
>>>
>>>> On Tue, Dec 1, 2009 at 3:18 PM, Fariborz Jahanian <fjahanian at apple.com
>>>>>
>>>> wrote:
>>>>>
>>>>> Author: fjahanian
>>>>> Date: Tue Dec  1 17:18:25 2009
>>>>> New Revision: 90283
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=90283&view=rev
>>>>> Log:
>>>>> Fix a code gen. crash synthesizing a destructor.
>>>>> Fixes pr5660.
>>>>
>>>> This patch looks very wrong.  I think the real issue here is
>>>> something
>>>> similar to PR5557: Sema doesn't know whether the vtable is created,
>>>> and therefore doesn't do the stuff that MarkDeclarationReferenced  
>>>> is
>>>> supposed to do for virtual functions.
>>>
>>> Yes, that is the problem. Patch just looks up the delete operator
>>> and makes it available in the destructor to be used, if synthesis is
>>> to occur. Can you point to what can go wrong?
>>
>> What if there isn't any valid delete operator?  I'm not entirely sure
>> off the top of my head how that would happen, but I think it's
>> possible.  We don't print a diagnostic, and there isn't a delete
>> operator, so we're back to crashing.
>
> I thought about this too. I think this will not happen. If we have a
> declaration which eventually requires
> the delete operator for its destruction, the CheckDestructor is called
> and  operator delete will
> be looked up. Except that this time a diagnostic will be issued.

Oops. It can happen. If user has a delete operator with extra arguments.

- Fariborz

>
> - fariborz
>
>>
>> -Eli
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits