[cfe-dev] Objective-C: methods, ivars, and cleanup

[David Chisnall]

On 25 Mar 2008, at 22:03, Devang Patel wrote:

>
> On Mar 25, 2008, at 10:00 AM, David Chisnall wrote:
>
>>> +  void CollectIvarTypes(ObjCInterfaceDecl *ObjCClass,
>>> +      std::vector<const llvm::Type*> *IvarTypes);
>>>
>>> This is Objective-C specific class method, so rename it as
>>> CollectObjCIvarTypes.
>>> Prefer SmallVector instead of std::vector. Pass it as a reference
>>> instead of a pointer.
>>
>> Fixed passing by pointer.  Kept as std::vector because
>> StructType::get() takes a std::vector.
>>
>>> /// getLLVMFieldNo - Return llvm::StructType element number
>>> /// that corresponds to the field FD.
>>> unsigned getLLVMFieldNo(const FieldDecl *FD);
>>> +  unsigned getLLVMFieldNo(QualType ObjectTy, const ObjCIvarDecl
>>> *Decl);
>>>
>>> Why do you need new method here ?
>>
>> This gets the field number from an LLVM structure created from an
>> Objective-C class, rather than one created from a C structure.
>> Possibly it should have a different name?  Suggestions welcome.
>
> In this method you do a field lookup by iterating over all ObjectTy
> ivars. Would not it be more efficient to keep a map just like normal C
> struct fields ?

You'd need to generate the map at some point, possibly the first time  
it's called?  Since it works as-is, I'd rather do this later if  
profiling indicates this is a bottleneck.

>>> +    // Warning: Use of this is strongly discouraged.  Late binding
>>> of instance
>>> +    // variables is supported on some runtimes and so using static
>>> binding can
>>> +    // break code when libraries are updated.  Only use this if
>>> you have
>>> +    // previously checked that the ObjCRuntime subclass in use
>>> does not support
>>> +    // late-bound ivars.
>>>
>>> Please add assert() to check late-bound of ivars support.
>>
>> I'd like to do this, but GodeGenTypes does not have a reference to
>> anything that has a reference to anything that has a reference to
>> the ObjCRuntime subclass, so it's non-trivial.  Suggestions welcome.
>
> You may want to collect runtime info while constructing CodeGenTypes.

Doing this would require major changes to the structure of  
CodeGenModule, since its Types member would have to be initialised  
after the runtime was constructed.  It's also not quite semantically  
correct - you should still be able to get the static structure of the  
object using @defs() at compile time, and the compiler should emit a  
warning that you are introducing stronger constraints on the ABI that  
the runtime requires (and thus susceptible to the fragile base class  
problem).

>>>> runtime_if.diff contains changes to the runtime interface.  This
>>>> probably needs to be committed at the same time as the changes to
>>>> the GNU runtime, or there might be some conflicts.  The Étoilé
>>>> patch may also conflict with the old definitions if it is
>>>> committed before this.
>>>
>>> OK
>>>
>>> -CGObjCRuntime *CreateObjCRuntime(llvm::Module &M);
>>> +CGObjCRuntime *CreateObjCRuntime(llvm::Module &M,
>>> +                                 const llvm::Type *LLVMIntType,
>>> +                                 const llvm::Type *LLVMLongType);
>>>
>>> Is not it possible to derive LLVMIntType and LLVMLongType info
>>> based on Module ?
>>
>> Maybe?  Can I have a hint please?
>
>
> IIUC, these are target specific int and long types. In that case,
> TargetInfo should provide you this info. I guess this OK for now.

This whole interface will probably be tweaked a few more times before  
it's finalised, but since it's only called in one place it's easy to  
fix later.

>> Thanks! Make sure while(AI) is not an infinite loop.

Well spotted.  It is now not an infinite loop...

David

-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang.diff
Type: application/octet-stream
Size: 46204 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20080326/38a28635/attachment.obj>
-------------- next part --------------