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

John McCall rjmccall at apple.com
Fri Nov 5 00:21:25 PDT 2010

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.


More information about the cfe-commits mailing list