[cfe-dev] [PATCH] Extend the use of DeclGroup

[Ted Kremenek]

Hi Zhongxing,

Thanks for taking this on!  This is a big patch, so I'm taking some  
time to go through it.  I had a quick question though about one thing  
that caught my attention:

  TranslationUnit::~TranslationUnit() {
+  for (std::vector<DeclGroupOwningRef>::reverse_iterator
+         I = TopLevelDecls.rbegin(), E = TopLevelDecls.rend(); I !=  
E; ++I) {
+    I->clear();
+  }
+
+#if 0
    if (OwnsDecls) {
      llvm::DenseSet<Decl*> Killed;
      for (std::vector<Decl*>::reverse_iterator  
I=TopLevelDecls.rbegin(),
@@ -102,7 +108,7 @@
        (*I)->Destroy(*Context);
      }
    }
-
+#endif

It looks like that in your patch that Decls will not be freed in  
~TranslationUnit since the "Destroy" method for each DeclGroupRef is  
not called.  Did you decide to do this because of issues with  
ownership?  If so, there needs to be a big FIXME comment here  
documenting the issue.  Actually, I don't think we should have a  
regression in functionality here (i.e., releasing Decls) unless it is  
absolutely necessary.  While the code in ~TranslationUnit that bends  
over backwards to avoid deleting a Decl more than once is gross, it  
also works, and IMHO shouldn't be removed until we have cleaned up the  
ownership issues in the AST.

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;
>
> 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.
>
> LinkageSpecDecl now uses a DeclGroup array to track all Decls.
>
> Three ObjC static analysis test cases fail. I haven't figured out  
> the reasons.
>
>
> <dg.patch>_______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev