[cfe-commits] r113397 - in /cfe/trunk: include/clang/AST/Type.h lib/AST/Type.cpp lib/Sema/SemaType.cpp test/SemaObjC/legacy-objc-types.m test/SemaObjCXX/legacy-objc-types.mm

[Fariborz Jahanian]

On Sep 8, 2010, at 2:59 PM, John McCall wrote:

> On Sep 8, 2010, at 2:30 PM, Fariborz Jahanian wrote:
>> On Sep 8, 2010, at 2:04 PM, John McCall wrote:
>>> If I understand this patch correctly, there's a crash arising from  
>>> invalid redeclarations of functions.  If that's true, this is  
>>> almost certainly a workaround rather than a fix, because it's just  
>>> making the redeclaration valid.  Please fix the underlying crash  
>>> rather than working around it like this.
>>
>> Underlying crash is in IRgen because we do not catch such duplicate  
>> function definitions. This patch makes the re-declaration   
>> identical to previous one and forces error in Sema.
>
> Oh, I see, because they mangle the same way.
>
> I think the appropriate place to make this change is in  
> Sema::FunctionArgTypesAreEqual, in SemaOverload.cpp.  We already  
> have similar logic for redeclaring functions with different protocol- 
> qualified types.

This was my first try :). Then I discovered that it prevents  
declaration followed by definition, as in:
id f(id ptr);
objc_object* f(id ptr) { }

So, the patch.

>
>
>>>
>> 1. Do nothing and let the test case crash ( I think it was a  
>> manufactured test ).
>> 2. Accept the alternative 'struct ...' as builtin type everywhere.
>> 3. Issue error if it is used anywhere.
>> 4.???
>
> Personally I like #3 because ObjC code using 'struct objc_object' is  
> very likely to either not compile or have really unfortunate  
> behavioral changes — I think Argyrios recently hunted down something  
> in the latter category — but that's the sort of problem we can only  
> suss out with SWBs.  For the immediate problem, changing  
> FunctionArgTypesAreEqual should be enough.
This is a policy decision. I am open to it.

- Fariborz

>
>
> John.