On Sep 8, 2010, at 3:10 PM, Fariborz Jahanian wrote:
>
> 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.
By 'prevents' do you mean that it produces an error or that it doesn't produce
an error and IR gen crashes on it? Because this *should* be an error unless we
start accepting objc_object as builtin-id.
>>> 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.
Okay. Can you file a radar to track this discussion?
John.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
