[cfe-dev] Fix for PR4701

[steve naroff]

On Aug 14, 2009, at 7:44 PM, David Chisnall wrote:

> On 15 Aug 2009, at 00:29, steve naroff wrote:
>
>>
>> On Aug 14, 2009, at 6:47 PM, David Chisnall wrote:
>>
>>> On 14 Aug 2009, at 23:24, steve naroff wrote:
>>>
>>>> From my perspective, depending on the C headers in this fashion  
>>>> is a big hack. When Objective-C was born, it wasn't such a big  
>>>> deal (since there was only one runtime).
>>>
>>> I completely agree.  It's a horrible hack, but that doesn't alter  
>>> the fact that a large body of existing code that depends on this  
>>> old behaviour has accumulated over the last two decades.
>>>
>>>> Now that id/Class are more 'builtin', Chris/I decided that  
>>>> ObjCIsaExpr worked nicely as a compatibility bridge.
>>>
>>> It's fine for id (well, almost; the GNU runtime calls this field  
>>> class_pointer, which is horrible but, again, something lots of  
>>> existing code depends on).  For Class / MetaClass, it's a much  
>>> bigger problem.
>>>
>>>> I just figured the same approach could be used for your  
>>>> situation. At Apple, we've totally moved away from allowing user  
>>>> code to directly access the meta-data. In our non-fragile  
>>>> runtime, all the same meta-data can be accessed using C functions  
>>>> (so we don't have this problem for Class).
>>>>
>>>> Unless you can deprecate some of this stuff (by replacing it with  
>>>> C function accessors), I'm afraid anything we do is a hack.
>>>
>>> For new code, that's fine.  I've written a compatibility framework  
>>> that sits on top of the GNU runtime and provides exactly the same  
>>> functionality with the same interfaces, but there is still a fair  
>>> body of legacy code for both the GNU runtime and the legacy Apple  
>>> runtime that directly manipulates the class structures and  
>>> currently clang can't compile this code, so some form of hack is  
>>> required.  We also need to maintain compatibility with GCC in a  
>>> lot of this code, which does require these headers for accessing  
>>> fields on id / Class / MetaClass.  Deprecating it is fine - and  
>>> I'd be really happy with a compiler switch that let us error on  
>>> code that attempted to access fields other than isa on these types  
>>> even with when their definitions are available - but deprecating  
>>> and removing are two different things.
>>>
>>> I've attached an updated version of the diff that fixes all of the  
>>> corner cases found by compiling GNUstep's Foundation.  It's a  
>>> hack, but it's a less-invasive hack than adding a load of new  
>>> expression types.  I think the ideal solution would be to have a  
>>> flag on typedef types that indicated that they can be used as  
>>> Objective-C message receivers.  I assumed  
>>> __attribute__((NSObject)) would do this, but it seems not to.  If  
>>> we had such a flag then we could just use id and Class typedefs if  
>>> they were provided with that flag set, which would provide an even  
>>> less-invasive hack.
>>>
>>
>> This diff seems too invasive. I think my suggestion would be much  
>> more localized.
>>
>> I'm still unclear why my suggestion doesn't fly (since it only  
>> involved renaming/repurposing the ObjCIsaExpr class...).
>>
>> How many fields are we talking about?
>
> For id, only one, although we need both ->isa and ->class_pointer  
> because we are slowly moving from using the old GNU headers (which  
> define it as ->class_pointer) to an Apple-compatible one.  For  
> Class, there are 13 in the GNU runtime and 10 for the legacy Apple  
> runtime.  Note that many of these have quite complex types  
> (structures containing arrays and so on).  If you can define them  
> all in a less-invasive change than the one I've proposed then please  
> commit it.
>

I see. I didn't realize the scope...

> Note that your proposal will leave us using two separate code paths  
> for accessing fields on struct objc_class* and fields on Class, in  
> spite of the two types being equivalent.
>
> Your proposal, however, still does not fix the MetaClass problem,  
> which requires more changes to allow implicit casts between Class  
> (which is now a builtin) and MetaClass (which is a typedef to the  
> same C structure that the headers define Class to be).   You will  
> also need to support all of the related cases, for example assigning  
> a struct objc_object* value to an id variable, assigning a Class to  
> a struct objc_class* variable.  Note that these are both permitted  
> even with the modern Apple runtime.  My diff handles this case  
> correctly.
>
> I believe once you have implemented equivalent functionality with  
> your proposal you will end up with a significantly more complex set  
> of changes than the ones I have written, but I would be more than  
> happy to be proven wrong, as long as something that compiles  
> existing Objective-C code is committed.
>

Given the number of fields, it sounds like we should go with your  
least disruptive solution to this. You've convinced me that my  
original suggestion is unlikely to scale to 23 fields...

Oh well. Given the amount of time I spent on this "cleanup", I hate to  
add cruft (but we need to support the legacy idioms I suppose...).

Thanks for your patience,

snaroff


> David