Hello Zhongxing,
Thanks for looking into this. I'm looking at your latest patch, but
I'm replying to your original message because I want to discuss a few
of the issues surrounding decl ownership. It's an area that's in flux
right now, and in some sense the DeclContext work I've been doing is
clashing with DeclGroups.
On Jan 4, 2009, at 1:10 AM, Zhongxing Xu wrote:
> This patch is the first step to solve the problem that no one is
> owning the
> struct decl in the following code:
>
> typedef struct s {} s_t;
The result that struct s { } isn't really owned here is because, at
present, we have two entities in the AST that are trying to own the
declarations in the translation unit, TranslationUnit and
TranslationUnitDecl, and the one that currently owns the declarations
in the translation unit doesn't know about "struct s { }".
With the typedef above, TranslationUnit's TopLevelDecls knows about
only "s_t", not "struct s", because TranslationUnit::AddTopLevelDecl
only gets called for "top-level" declarations.
TranslationUnitDecl, on the other hand, is a DeclContext that knows
about all declarations in the scope of the translation unit, including
both "struct s" and "s_t". (It has to know about all declarations,
because it's used for name lookup).
My intent with DeclContexts is that a DeclContext should own all of
the declarations within that context. So TranslationUnitDecl (which is
a DeclContext) should own both "struct s" and "s_t". The problem, in
our current state, is that TranslationUnit also tries to own top-level
decls (currently, "s_t" but not "struct s") through its TopLevelDecls
member, and DeclGroups try to own declarations in a declaration
statement (e.g., "x" and "y" in "void f() { int x, y; }"), leading to
this insanely gross hack in the serialization of DeclContexts (see
DeclContext::EmitOutRec):
bool Owned = ((*D)->getLexicalDeclContext() == this &&
DeclKind != Decl::TranslationUnit &&
!isFunctionOrMethod());
What's that's saying is, basically, DeclContext owns its decls except
in the cases where we have DeclGroups. That's the clash between the
two mechanisms that I mentioned above.
I was working toward a solution that would change DeclGroups and
TranslationUnit so that they don't own any decls. Instead, DeclContext
would handle all of the ownership issues. The benefit of this is that
there are many places where we have DeclContexts already managing
ownership of their decls (namespaces, classes, linkage specifications,
and enumerations come to mind), and all of those places could benefit
from having support for DeclGroups to describe the syntax the user
wrote.
Trying to summarize: I think DeclContext should handle all of the
ownership issues, DeclGroups should handle representing the syntax,
and I'd like the two to work together.
>
> Later a TypeSpecifier holding this struct decl will be added to
> DeclGroup as
> discussed on the mailing list months before.
>
> The root of changes is at Sema::FinalizeDeclaratorGroup. It returns
> DeclGroupOwningRef instead of DeclTy*. A bunch of related methods
> now return
> DeclGroupOwningRef instead of DeclTy*.
>
> Top level declarations are DeclGroups instead of Decls.
> TranslationUnit
> uses a vector of DeclGroupOwningRef to track all Decls. Most of the
> dtor of
> TranslationUnit is disabled. The cleanup works should be done by the
> DeclGroup.
Ah, this is the most direct place where we clash. Your patch updates
TranslationUnit's vector (TopLevelDecls) to store DeclGroups rather
than Decls (which is good), but my plan for DeclContexts was to
eliminate TranslationUnit::TopLevelDecls entirely: clients that want
to iterate through the declarations in a translation unit would,
instead, use TranslationUnitDecl's decls_begin()/decls_end().
My hunch is that what we want is to find a way to iterate through a
DeclContext in a way that allows us to see the DeclGroups (or, at
least, see the syntactic relationship between "struct s { }" and
"s_t"). However, I'd like to hear your thoughts on these ownership and
representation issues.
- Doug