[cfe-commits] r69980 - in /cfe/trunk: lib/Sema/SemaExpr.cpp test/SemaObjC/sizeof-interface.m

[Fariborz Jahanian]

On Apr 24, 2009, at 3:32 PM, Chris Lattner wrote:

> On Apr 24, 2009, at 11:53 AM, Fariborz Jahanian wrote:
>>> This patch looks wrong; this should definitely be returning true.
>>> Otherwise, it looks like a valid declaration.
>>
>> But we are issuing the diagnostic and then return true.
>
> this is both a purity and error recovery issue.  The purity issue  
> here is that we should never create an invalid AST unless it is a  
> decl that is marked invalid.
>
>>>> +    @public
>>>> +    unsigned long attributeRuns[1024 + sizeof(I)]; // expected-
>>>> error {{invalid application of 'sizeof' to interface 'I' in non-
>>>> fragile ABI}}
>>>
>>> Specifically, have you considered what happens when someone tries to
>>> compute sizeof(attributeRuns)?
>>
>> Intention is to not issue additional errors.
>> We have already issued the diagnostic on the declaration. But, I
>> understand, that
>> this may be contentious (project I was concerned with issued 100s of
>> additional
>> diagnostics).
>
> r69998 fixes this a different way by propagating the 'ininvalid' bit  
> better, and making sure that uses of the invalid decl don't get  
> chains of error messages.  This fixes the testcase, does it also fix  
> the projects you are working with?
Yes, thanks. It fixes the project in question.

- Fariborz

>
>
> Thanks for tackling this, I really appreciate the attention to error  
> recovery!
>
> -Chris
>