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

[Eli Friedman]

On Tue, Dec 1, 2009 at 7:33 PM, Fariborz Jahanian <fjahanian at apple.com> 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.

If it's in the vtable, the destructor might be called, even if the
call isn't obvious.  That isn't really the issue here anyway, though:
the issue is that it would crash trying to generate the destructor,
whether or not it's actually called.

-Eli