Remove the TopLevelDecls from TranslationUnit, since all of those decls are owned by the ASTContext's TranslationUnitDecl. There are definitely some leaking Decls now that I'll tackle tomorrow

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@62568 91177308-0d34-0410-b5e6-96231b3b80d8

7 files changed, 16 insertions, 131 deletions

diff --git a/include/clang/AST/TranslationUnit.h b/include/clang/AST/TranslationUnit.h
index 2153a0b1eb..9c356a20fd 100644
--- a/include/clang/AST/TranslationUnit.h
+++ b/include/clang/AST/TranslationUnit.h
@@ -14,9 +14,9 @@
 #define LLVM_CLANG_TRANSLATION_UNIT_H
 
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
 #include "llvm/Bitcode/SerializationFwd.h"
 #include "llvm/System/Path.h"
-#include <vector>
 #include <string>
 
 namespace clang {
@@ -32,7 +32,6 @@ class FileEntry;
   
 class TranslationUnit {
   ASTContext* Context;
-  std::vector<Decl*> TopLevelDecls;
   bool OwnsMetaData;
   bool OwnsDecls;
 
@@ -61,20 +60,12 @@ public:
 
   ASTContext&        getContext() { return *Context; }
   const ASTContext&  getContext() const { return *Context; }
-  
-  /// AddTopLevelDecl - Add a top-level declaration to the translation unit.
-  ///  Ownership of the Decl is transfered to the TranslationUnit object.
-  void AddTopLevelDecl(Decl* d) {
-    TopLevelDecls.push_back(d);
+
+  typedef DeclContext::decl_iterator iterator;
+  iterator begin() const { 
+    return Context->getTranslationUnitDecl()->decls_begin(); 
   }
-  
-  typedef std::vector<Decl*>::iterator iterator;  
-  iterator begin() { return TopLevelDecls.begin(); }
-  iterator end() { return TopLevelDecls.end(); }
-  
-  typedef std::vector<Decl*>::const_iterator const_iterator;  
-  const_iterator begin() const { return TopLevelDecls.begin(); }
-  const_iterator end() const { return TopLevelDecls.end(); }  
+  iterator end() const { return Context->getTranslationUnitDecl()->decls_end(); }
 };
   
 /// EmitASTBitcodeFile - Emit a translation unit to a bitcode file.
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index 9ddc62a51e..165a6c0948 100644
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -319,7 +319,6 @@ RecordDecl::~RecordDecl() {
 }
 
 void RecordDecl::Destroy(ASTContext& C) {
-  DeclContext::DestroyDecls(C);
   TagDecl::Destroy(C);
 }
 
diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp
index 70792c4efd..5526be091f 100644
--- a/lib/AST/DeclBase.cpp
+++ b/lib/AST/DeclBase.cpp
@@ -358,25 +358,26 @@ void Decl::swapAttrs(Decl *RHS) {
 
 void Decl::Destroy(ASTContext& C) {
 #if 0
-  // FIXME: This causes double-destroys in some cases, so it is
-  // disabled at the moment.
+  // FIXME: Once ownership is fully understood, we can enable this code
+  if (DeclContext *DC = dyn_cast<DeclContext>(this))
+    DC->decls_begin()->Destroy(C);
 
-  // Observe the unrolled recursion.  By setting N->NextDeclarator = 0x0
+  // Observe the unrolled recursion.  By setting N->NextDeclInScope = 0x0
   // within the loop, only the Destroy method for the first Decl
   // will deallocate all of the Decls in a chain.
   
-  Decl* N = SD->getNextDeclarator();
+  Decl* N = NextDeclInScope;
   
   while (N) {
-    Decl* Tmp = N->getNextDeclarator();
-    N->NextDeclarator = 0x0;
+    Decl* Tmp = N->NextDeclInScope;
+    N->NextDeclInScope = 0;
     N->Destroy(C);
     N = Tmp;
   }  
-#endif
 
   this->~Decl();
   C.getAllocator().Deallocate((void *)this);
+#endif
 }
 
 Decl *Decl::castFromDeclContext (const DeclContext *D) {
@@ -427,14 +428,8 @@ DeclContext::~DeclContext() {
 }
 
 void DeclContext::DestroyDecls(ASTContext &C) {
-  for (decl_iterator D = decls_begin(); D != decls_end(); ) {
-   // FIXME: assert that this condition holds.
-   if ((*D)->getLexicalDeclContext() == this)
-     // Advance the cursor (via NextDeclInScope) *before* doing the Destroy.
-     (*D++)->Destroy(C);
-   else
-     ++D;
-  }
+  for (decl_iterator D = decls_begin(); D != decls_end(); )
+    (*D++)->Destroy(C);
 }
 
 bool DeclContext::isTransparentContext() const {
diff --git a/lib/AST/TranslationUnit.cpp b/lib/AST/TranslationUnit.cpp
index c02db82dab..9e01167248 100644
--- a/lib/AST/TranslationUnit.cpp
+++ b/lib/AST/TranslationUnit.cpp
@@ -31,78 +31,6 @@ enum { BasicMetadataBlock = 1,
        DeclsBlock = 3 };
 
 TranslationUnit::~TranslationUnit() {
-  if (OwnsDecls) {
-    llvm::DenseSet<Decl*> Killed;
-    for (std::vector<Decl*>::reverse_iterator I=TopLevelDecls.rbegin(), 
-                                              E=TopLevelDecls.rend(); 
-         I!=E; ++I) {
-      if (Killed.count(*I)) continue;
-
-      Killed.insert(*I);
-      
-      // FIXME: This is a horrible hack.  Because there is no clear ownership
-      //  role between ObjCInterfaceDecls and the ObjCPropertyDecls that they
-      //  reference, we need to destroy ObjCPropertyDecls here.  This will
-      //  eventually be fixed when the ownership of ObjCPropertyDecls gets
-      //  cleaned up.
-      if (ObjCInterfaceDecl* IDecl = dyn_cast<ObjCInterfaceDecl>(*I))
-        for (ObjCInterfaceDecl::prop_iterator ID=IDecl->prop_begin(),
-             ED=IDecl->prop_end(); ID!=ED; ++ID) {
-          if (!*ID || Killed.count(*ID)) continue;
-          Killed.insert(*ID);
-          (*ID)->Destroy(*Context);
-        }
-      
-      // FIXME: This is a horrible hack.  Because there is no clear ownership
-      //  role between ObjCProtocolDecls and the ObjCPropertyDecls that they
-      //  reference, we need to destroy ObjCPropertyDecls here.  This will
-      //  eventually be fixed when the ownership of ObjCPropertyDecls gets
-      //  cleaned up.
-      if (ObjCProtocolDecl* PDecl = dyn_cast<ObjCProtocolDecl>(*I))
-        for (ObjCProtocolDecl::prop_iterator ID=PDecl->prop_begin(),
-             ED=PDecl->prop_end(); ID!=ED; ++ID) {
-          if (!*ID || Killed.count(*ID)) continue;
-          Killed.insert(*ID);
-          (*ID)->Destroy(*Context);
-        }
-            
-      // FIXME: There is no clear ownership policy now for ObjCInterfaceDecls
-      //  referenced by ObjCClassDecls.  Some of them can be forward decls that
-      //  are never later defined (and forward decls can be referenced by
-      //  multiple ObjCClassDecls) or the ObjCInterfaceDecl later
-      //  becomes a real definition. 
-      //  Ideally we should have separate objects for forward declarations and
-      //  definitions, obviating this problem.  Because of this situation,
-      //  referenced ObjCInterfaceDecls are destroyed here.      
-      if (ObjCClassDecl* CDecl = dyn_cast<ObjCClassDecl>(*I))
-        for (ObjCClassDecl::iterator ID=CDecl->begin(),
-             ED=CDecl->end(); ID!=ED; ++ID) {          
-          if (!*ID || Killed.count(*ID)) continue;
-          Killed.insert(*ID);
-          (*ID)->Destroy(*Context);
-        }
-      
-      // FIXME: There is no clear ownership policy now for ObjCProtocolDecls
-      //  referenced by ObjCForwardProtocolDecl.  Some of them can be forward 
-      //  decls that are never later defined (and forward decls can be
-      //  referenced by multiple ObjCClassDecls) or the ObjCProtocolDecl 
-      //  later becomes a real definition. 
-      //  Ideally we should have separate objects for forward declarations and
-      //  definitions, obviating this problem.  Because of this situation,
-      //  referenced ObjCProtocolDecls are destroyed here.  
-      if (ObjCForwardProtocolDecl* FDec = dyn_cast<ObjCForwardProtocolDecl>(*I))
-        for (ObjCForwardProtocolDecl::iterator ID=FDec->begin(),
-             ED=FDec->end(); ID!=ED; ++ID) {          
-          if (!*ID || Killed.count(*ID)) continue;
-          Killed.insert(*ID);
-          (*ID)->Destroy(*Context);
-        }
-      
-            
-      (*I)->Destroy(*Context);
-    }
-  }
-
   if (OwnsMetaData && Context) {
     // The ASTContext object has the sole references to the IdentifierTable
     // Selectors, and the Target information.  Go and delete them, since
@@ -192,22 +120,6 @@ bool clang::EmitASTBitcodeFile(const TranslationUnit& TU,
 }
 
 void TranslationUnit::Emit(llvm::Serializer& Sezr) const {
-
-  // ===---------------------------------------------------===/
-  //      Serialize the top-level decls.
-  // ===---------------------------------------------------===/  
-  
-  Sezr.EnterBlock(DeclsBlock);
-
-  // Only serialize the head of a decl chain.  The ASTConsumer interfaces
-  // provides us with each top-level decl, including those nested in
-  // a decl chain, so we may be passed decls that are already serialized.  
-  for (const_iterator I=begin(), E=end(); I!=E; ++I) 
-      if (!Sezr.isRegistered(*I))
-        Sezr.EmitOwnedPtr(*I);
-  
-  Sezr.ExitBlock();
-  
   // ===---------------------------------------------------===/
   //      Serialize the "Translation Unit" metadata.
   // ===---------------------------------------------------===/
@@ -333,15 +245,6 @@ TranslationUnit* TranslationUnit::Create(llvm::Deserializer& Dezr,
   Dezr.JumpTo(ASTContextBlockLoc);
   TU->Context = Dezr.ReadOwnedPtr<ASTContext>();
   
-  // "Rewind" the stream.  Find the block with the serialized top-level decls.
-  Dezr.Rewind();
-  FoundBlock = Dezr.SkipToBlock(DeclsBlock);
-  assert (FoundBlock);
-  llvm::Deserializer::Location DeclBlockLoc = Dezr.getCurrentBlockLocation();
-  
-  while (!Dezr.FinishedBlock(DeclBlockLoc))
-    TU->AddTopLevelDecl(Dezr.ReadOwnedPtr<Decl>(*TU->Context));
-
   return TU;
 }
 
diff --git a/lib/Sema/ParseAST.cpp b/lib/Sema/ParseAST.cpp
index 272615cceb..ff6de25c1d 100644
--- a/lib/Sema/ParseAST.cpp
+++ b/lib/Sema/ParseAST.cpp
@@ -58,7 +58,6 @@ void clang::ParseAST(Preprocessor &PP, ASTConsumer *Consumer,
     // skipping something.
     if (ADecl) {
       Decl* D = static_cast<Decl*>(ADecl);      
-      TU->AddTopLevelDecl(D); // TranslationUnit now owns the Decl.
       Consumer->HandleTopLevelDecl(D);
     }
   };
diff --git a/test/Serialization/complex.c b/test/Serialization/complex.c
index 1d9327bad1..f622264842 100644
--- a/test/Serialization/complex.c
+++ b/test/Serialization/complex.c
@@ -1,5 +1,4 @@
 // RUN: clang %s --test-pickling 2>&1 | grep -q 'SUCCESS'
-// XFAIL
 
 int main(void)
 {
diff --git a/test/Serialization/stmt_exprs.c b/test/Serialization/stmt_exprs.c
index 0c30be2b56..9ee242fd11 100644
--- a/test/Serialization/stmt_exprs.c
+++ b/test/Serialization/stmt_exprs.c
@@ -1,5 +1,4 @@
 // RUN: clang %s --test-pickling 2>&1 | grep -q 'SUCCESS'
-// XFAIL
 typedef unsigned __uint32_t;
 
 #define __byte_swap_int_var(x) \