On 06/30/2011 03:56 AM, Richard Guenther wrote:
>> > It is only used by darwin at present - so nothing is split out for any
>> > other
>> > target (the default hook is simply 'false').
> Yes, I've seen that. Still a hook like this should be generally useful,
> and you still process through some pieces of assemble_variable
> while you choose to skip some other piece - it should probably be
> documented which part of assemble_variable is supposed to be handled
> by the hook.
>
> I'll defer to Richard for this (and the approval).
>
I definitely want to see better documentation. I don't know how to
review the patch at the moment. Some background for us non-darwin
programmers would help as well; I don't know what's legal and what
isn't when it comes to these sorts of odd non-definitions.
You're changing the set of variables emitted for non-darwin here
too, as far as I can tell. Is that actually desired? I can't
imagine what these variables are intended to achieve, given that
they're byte sized and DECL_INITIAL = error_mark_node.
I can say that you need to watch the formatting. There are missing
spaces after commas, and incorrect indentation.
> + if (DECL_ATTRIBUTES (decl)
> + && (meta = lookup_attribute ("OBJC1META", DECL_ATTRIBUTES (decl))))
Do not assign to variables inside if conditions. No need to check
for DECL_ATTRIBUTES non-null before calling lookup_attribute.