[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
fjahanian at apple.com
Wed Sep 8 15:10:20 PDT 2010
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.
More information about the cfe-commits
mailing list