[cfe-commits] [PATCH][MS][Review request] - Microsoft C anonymous struct, second try

[Douglas Gregor]

On Nov 8, 2010, at 11:35 PM, Francois Pichet wrote:

> On Fri, Nov 5, 2010 at 3:21 AM, John McCall <rjmccall at apple.com> wrote:
>> On Nov 4, 2010, at 12:24 PM, Francois Pichet wrote:
>>> Anybody planning to review that?
>> 
>> Sorry, I should have reviewed this earlier.
>> 
>> This approach seems pretty fragile — which is not your fault, it's just that our history with anonymous structs/unions has been that they've been subject to a long succession of bugs due to poor AST representation, and I foresee the same applying here, particularly since the feature is specific to a platform that most contributors aren't testing with.  So I'd like to counter-propose an idea, and if you aren't certain how to implement this, let me know and we can try to figure something out.
>> 
>> The commonality between this feature and anonymous structs/unions is that a name is added to a struct which actually refers to a member of a subobject of that struct.  It seems to me that the best "unifying" approach would be to add some sort of NestedFieldDecl (IndirectFieldDecl?  anyway, a subclass of ValueDecl) which consists of a chain of two or more declarations.  When injecting fields for a GNU/C++ anonymous structs and union, for each field we would inject a NestedFieldDecl whose path would terminate in the appropriate field;  similarly, for MS anonymous structs, for each field of that struct we would inject a NestedFieldDecl leading to it.  All the code in member access, etc., which checks for a member of an anonymous struct or union would instead check for a NestedFieldDecl and expand it out into the same sequence of accesses that we expand into today.
>> 
>> For top-level anonymous structs or unions, the first declaration in a NestedFieldDecl's chain would be a VarDecl;  otherwise it would be a FieldDecl.  In all cases, all the remaining decls would be FieldDecls.
>> 
>> I think this new representation would naturally support both of these features in a way that would be straightforward to correctly support.  Let me know what you think and, if you approve, whether you think you're ready to implement this sort of deep change to the AST.
>> 
>> John.
> 
> Hi I am currently implementing this.
> 
> 1. I prefer IndirectFieldDecl to NestedFieldDecl. is that ok?

Sounds great to me!

> 2. I think IndirectFieldDecl should derive from FieldDecl instead of
> ValueDecl. Are you ok with this?

I'd rather not do this...

> The reason is that currently a name lookup on an anonymous field
> results in a FieldDecl. After my patch it will result in an
> IndirectFieldDecl. Some places in the code expect a FieldDecl after a
> name lookup (ie: BuildMemberInitializer). Having IndirectFieldDecl
> derives from FieldDecl seems logical and let an IndirectFieldDecl to
> impersonate a FieldDecl which is what we want.

for exactly this reason. If IndirectFieldDecl isa FieldDecl, we're liable to accidentally treat IndirectFieldDecl as a FieldDecl and, by doing so, screw up our handling of anonymous struct/union members in Sema and CodeGen. By making it a separate ValueDecl, we force ourselves to deal with this specific construct appropriately. (And it'll be easy to tell when we fall through to a general case)

	- Doug