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

[Chris Lattner]

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?

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

-Chris