[cfe-dev] Fix for PR4701

[steve naroff]

On Aug 14, 2009, at 6:03 PM, David Chisnall wrote:

> On 14 Aug 2009, at 22:51, steve naroff wrote:
>
>>
>> On Aug 14, 2009, at 5:21 PM, David Chisnall wrote:
>>
>>> Hi Steve,
>>>
>>> there's a test case attached to PR4701 (http://llvm.org/bugs/show_bug.cgi?id=4701 
>>> ).  I've just discovered that this still breaks assigning Class  
>>> variables to struct objc_class* variables and vice versa.  I think  
>>> I can fix this by adding another hack in  
>>> Sema::CheckPointerTypesForAssignment(), but I'd still prefer a  
>>> better approach if you can suggest one...
>>>
>>
>> Why not replace 'ObjCIsaExpr' with 'ObjCBuiltinFieldExpr' (used for  
>> both 'id' and 'Class'...)? You would need to add an explicit  
>> 'IdentifierInfo' slot...
>
> I'm not sure what this would achieve.  You'd then need to hard-code  
> all of the fields of Class into the compiler.  For future OS X  
> compatibility, we are slowly moving towards using a replacement  
> header which defines Class as an opaque type, and this approach  
> would make these fields visible even when the header is not  
> included, which is not what we want.  It would also complicate Sema  
> a lot because we'd have to add a set of ObjCBuiltinFieldExpr nodes  
> for each field in each runtime we support (currently the GNU runtime  
> and the Apple legacy runtime would need a set of these).
>

 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).

Now that id/Class are more 'builtin', Chris/I decided that ObjCIsaExpr  
worked nicely as a compatibility bridge. 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.

snaroff

>> Would this do the trick?
>
> I also don't see how this would help with casts from Class to  
> MetaClass and vice versa either.
>
> David
>
>> snaroff
>>
>>> When I try to compile GNUstep, I discover a more subtle problem  
>>> which my patch now fixes, which is that MetaClass is defined like  
>>> this:
>>>
>>> typedef struct objc_class *MetaClass;
>>>
>>> This means that clang fails to notice that MetaClass and Class  
>>> (which are both typedefs for the same underlying type) are  
>>> compatible when using the builtin definition of Class, which  
>>> breaks any assignment of a MetaClass value to a Class-typed  
>>> variable and vice versa.
>>>
>>> David
>>>
>>> On 14 Aug 2009, at 22:08, steve naroff wrote:
>>>
>>>> Hi David,
>>>>
>>>> If you can include a test case that demonstrate what this patch  
>>>> fixes, I believe I can give you a better approach...
>>>>
>>>> Thanks,
>>>>
>>>> snaroff
>>>>
>>>> On Aug 14, 2009, at 4:56 PM, David Chisnall wrote:
>>>>
>>>>> Hi Everyone,
>>>>>
>>>>> This patch provides an actual fix (as opposed to my previous  
>>>>> attempt, which just moved the bugs around) for PR4701, where  
>>>>> code accessing fields declared on Objective-C pseudo-builtin  
>>>>> types id and Class broke.
>>>>>
>>>>> With this patch applied, all test pass and programs that include  
>>>>> the GNU runtime's <obj/objc-api.h> compile correctly.
>>>>>
>>>>> I am not totally convinced by the approach here.  If a  
>>>>> redefinition of id or Class is encountered, this is stored any  
>>>>> any attempt to access a field on one of these builtins is then  
>>>>> implicitly cast to the real type.  This is quite ugly, but seems  
>>>>> less ugly than doing it the other way around (adding special  
>>>>> cases to all of the Objective-C stuff to let struct objc_object*  
>>>>> act as a receiver, and so on).
>>>>>
>>>>> David
>>>>> <clang.diff>_______________________________________________
>>>>> cfe-dev mailing list
>>>>> cfe-dev at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>>>
>>>
>>
>