[cfe-dev] Cleaning up the representation of Decls in the AST

[Ted Kremenek]

On Sep 11, 2008, at 5:53 AM, Argiris Kirtzidis wrote:

> Ted Kremenek wrote:
>> Note that this completely resolves the ownership issues with  
>> anonymous
>> structs and similar constructs.  It also means that Types no longer
>> own Decls (a good thing).
>>
>
> There's an ownership issue that is not mentioned:
>
> x = sizeof( struct { int z; } );   // who owns this RecordDecl ?
>
> This may indicate that RecordDecls are fundamentally part of Type  
> and should be owned by a Type.

Hi Argiris,

This is an excellent point.  There are a couple reasons why I strongly  
don't believe that types should ever own elements of the AST.

 From an aesthetic perspective, declarations represent program syntax,  
while types are part of the program semantics.  I don't believe our  
data structures representing semantics should own the data structures  
that represent program syntax.  They can refer to each other, but  
there isn't an ownership relationship here.  Conceptually it just  
doesn't make much sense, and I believe that one of the goals of the  
ASTs is that they should be conceptually clean as possible.  This  
allows clients of the ASTs to understand their invariants much more  
easily.  This was actually one of the prime motivation of the  
DeclGroup idea that Daniel and I put forth.  Beyond cleaning up some  
ownership issues, DeclGroups really do capture more elements of C's  
actually syntax.

Another reason that having types own Decls is that it really makes  
things much more difficult for clients that wish to modify the AST.   
When is a Decl owned by a type?  With the idea you propose, a DeclStmt  
might own a TypeDecl if it isn't a RecordDecl, but in the case of a  
RecordType the type would own it.  There is also the problem that we  
now have multiple RecordDecls for a given RecordType.  Does the  
RecordType own all of those RecordDecls?  This actually makes  
manipulation of the AST really tricky and error prone.

 From my perspective, Decls just represent syntax, and if our current  
ASTs cannot adequately capture such elements of the syntax then they  
need to be augmented.  In the case of sizeof(type), it seems to me  
that a reasonable solution would be to have SizeOfAlignOfTypeExpr  
refer to a variant instead of a QualType.  In the common case the  
variant would just refer to a QualType, and in the other case it would  
refer to both a QualType and a TypeDecl (in which case it owns the  
TypeDecl).  Using either smart pointers or subclassing of  
SizeOfAlignOfTypeExpr, it seems that the storage of such extra  
information could readily be incorporated into the AST.   
getArgumentType() could then be made virtual (if necessary) to query  
the appropriate field.

The nice thing about this solution is that it isolates the ugliness of  
C's grammar for this particular problem into a few bits of code in the  
AST, rather than complicating the representation of the C type system  
or introducing hard to reason about ownership issues between Types and  
Decls.  Naturally the parser would need to be augmented to reason  
about variants as well; i.e., in the case of sizeof(type), the logic  
in the parser that returns the QualType for "type" actually may return  
a "type" or a "decl".  This seems fine to me; we're already passing  
variants around in the parser.  Another thing that's nice about this  
solution is that IMHO it is conceptually clean; the fact the sizeof  
can refer to a type or a decl is just a consequence of C's grammar  
that is represented faithfully in the AST itself; clients that care  
about whether or not sizeof(type) actually refers to a declaration  
will need to be able to distinguish between these cases, and other  
clients that just want the type that sizeof(type) refers to won't have  
to care at all.

There will likely be other corner cases like sizeof(type) that we will  
need to handle, but these all seem to me to be issues of syntax that  
require enhancements to the AST themselves.  Having the RecordTypes  
own RecordDecls really just seems like a short-term solution that  
muddles the conceptual cleanliness of the ASTs and introduces  
artificial ownership relationships between types and decls that we  
would only think about introducing in order to solve gross corner  
cases like these.  It just doesn't feel like the right solution to me.

Ted