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.