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

[Zhongxing Xu]

I agree with Ted that we should separate syntax thing from semantics thing
in the AST.

struct s;
struct s a;
struct s { int d; } x;

These 'struct s' have different meanings: type declaration, type specifier,
type definition.
But syntactically they are all RecordDecl.

Having Type owns Decls introduces confusion.

-Zhongxing Xu

2008/9/12 Argiris Kirtzidis <akyrtzi at gmail.com>

> Ted Kremenek wrote:
> >
> > 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.
>
> To be more clear, the idea is that TypeDecls are owned by Types, and
> only one RecordDecl would be created.
> Can clients modify the AST without involving Types ? Assuming one
> RecordDecl in the AST is replaced with another, what about the
> RecordType the refers to it ?
> My point is that if the DeclGroup owns the RecordDecl, the Type will
> still need to be updated. If the Type owns the the RecordDecl you may
> only need to update the Type.
>
> You also mention that:
>
> > - Source-Fidelity: We capture much more of the source information in
> >   the ASTs.  This is great for refactoring clients.
>
> The way I see it, DeclGroups mostly solve ownership issues, it's not a
> "real" solution for refactoring clients. These clients would want all
> type references too, not just forward declarations and definitions.
> The only way to support refactoring clients seems to be the proposal by
> Chris here:
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2008-August/002614.html
> which is a kind of "Type AST" thing, so complicating the type system may
> be inevitable. TypeDecls may be part of that "Type AST".
>
> -Argiris
>
>
> >
> > 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
> >
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20080912/300b3e6e/attachment.html>