<br><br><div class="gmail_quote">On Wed, Jan 7, 2009 at 10:33 AM, Douglas Gregor <span dir="ltr"><<a href="mailto:dgregor@apple.com">dgregor@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hello Zhongxing,<br>
<br>
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.<div class="Ih2E3d">
<br>
<br>
On Jan 4, 2009, at 1:10 AM, Zhongxing Xu wrote:<br>
<br>
</div><div class="Ih2E3d"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
This patch is the first step to solve the problem that no one is owning the<br>
struct decl in the following code:<br>
<br>
typedef struct s {} s_t;<br>
</blockquote>
<br></div>
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 { }".<br>

<br>
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.<br>

<br>
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).<br>

<br>
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):<br>

<br>
    bool Owned = ((*D)->getLexicalDeclContext() == this &&<br>
                  DeclKind != Decl::TranslationUnit &&<br>
                  !isFunctionOrMethod());<br>
<br>
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.<br>
<br>
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.<br>

<br>
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.<div class="Ih2E3d"><br>
<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
Later a TypeSpecifier holding this struct decl will be added to DeclGroup as<br>
discussed on the mailing list months before.<br>
<br>
The root of changes is at Sema::FinalizeDeclaratorGroup.  It returns<br>
DeclGroupOwningRef instead of DeclTy*.  A bunch of related methods now return<br>
DeclGroupOwningRef instead of DeclTy*.<br>
<br>
Top level declarations are DeclGroups instead of Decls.  TranslationUnit<br>
uses a vector of DeclGroupOwningRef to track all Decls.  Most of the dtor of<br>
TranslationUnit is disabled. The cleanup works should be done by the DeclGroup.<br>
</blockquote>
<br></div>
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().<br>

<br>
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.<br>

<br>
        - Doug<br>
</blockquote></div><br>Hi Doug,<br><br>I'll take a deep look at all the DeclContext stuff and mechanism in the following days. But my feeling is that DeclContext should be the ultimate solution to the ownership problem. I'll wait for your progress in this area.<br>
<br>On the other hand, as you said, we still need a mechanism to recover the original syntax structure of the source (and more ideally, the layout of the source code). I think the DeclGroup could play that role with the following structure:<br>
                      DeclGroup => TypeSpecifier Decls<br><br>Instead of owning the decls, the DeclGroup and TypeSpecifier could only refer to the decls with a pointer.<br><br>Also the TopLevelDecls in TranslationUnit probably could be kept to reflect the code layout?<br>
<br>- Zhongxing<br>