[cfe-dev] Objective-C: methods, ivars, and cleanup
dpatel at apple.com
Wed Mar 26 09:40:21 PDT 2008
On Mar 26, 2008, at 8:09 AM, David Chisnall wrote:
>> 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?
The map is collected for C structure while determining its layout (in
> Since it works as-is, I'd rather do this later if profiling
> indicates this is a bottleneck.
A class is defined once but its fields are referenced multiple times,
so it makes sense to avoid lookups during field access. That's what we
do for C structures. Let's not diverge here.
> + assert(!verifyFunction(*CurFn));
nit-pick, add a message here.
>>>> + // 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).
hmm.. OK for now.
Otherwise patch looks OK. Very nice!
More information about the cfe-dev