Change struct forward declarations and definitions to use unique RecordDecls, as opposed to creating a single RecordDecl and reusing it.

This change effects both RecordDecls and CXXRecordDecls, but does not effect EnumDecls (yet).

The motivation of this patch is as follows:
- Capture more source information, necessary for refactoring/rewriting clients.

- Pave the way to resolve ownership issues with RecordDecls with the forthcoming
  addition of DeclGroups.

Current caveats:
- Until DeclGroups are in place, we will leak RecordDecls not explicitly
  referenced by the AST.  For example:

    typedef struct { ... } x;  

  The RecordDecl for the struct will be leaked because the TypedefDecl doesn't
  refer to it.  This will be solved with DeclGroups.
  
- This patch also (temporarily) breaks CodeGen.  More below.

High-level changes:
- As before, TagType still refers to a TagDecl, but it doesn't own it.  When
  a struct/union/class is first referenced, a RecordType and RecordDecl are
  created for it, and the RecordType refers to that RecordDecl.  Later, if
  a new RecordDecl is created, the pointer to a RecordDecl in RecordType is
  updated to point to the RecordDecl that defines the struct/union/class.

- TagDecl and RecordDecl now how a method 'getDefinition()' to return the
  TagDecl*/RecordDecl* that refers to the TagDecl* that defines a particular
  enum/struct/class/union. This is useful from going from a RecordDecl* that
  defines a forward declaration to the RecordDecl* that provides the actual
  definition. Note that this also works for EnumDecls, except that in this case
  there is no distinction between forward declarations and definitions (yet).

- Clients should no longer assume that 'isDefinition()' returns true from a
  RecordDecl if the corresponding struct/union/class has been defined.
  isDefinition() only returns true if a particular RecordDecl is the defining
  Decl. Use 'getDefinition()' instead to determine if a struct has been defined.

- The main changes to Sema happen in ActOnTag. To make the changes more
  incremental, I split off the processing of enums and structs et al into two
  code paths. Enums use the original code path (which is in ActOnTag) and
  structs use the ActOnTagStruct. Eventually the two code paths will be merged,
  but the idea was to preserve the original logic both for comparison and not to
  change the logic for both enums and structs all at once.

- There is NO CHAINING of RecordDecls for the same RecordType. All RecordDecls
  that correspond to the same type simply have a pointer to that type. If we
  need to figure out what are all the RecordDecls for a given type we can build
  a backmap.

- The diff in CXXRecordDecl.[cpp,h] is actually very small; it just mimics the
  changes to RecordDecl. For some reason 'svn' marks the entire file as changed.

Why is CodeGen broken:
- Codegen assumes that there is an equivalence between RecordDecl* and
  RecordType*. This was true before because we only created one RecordDecl* for
  a given RecordType*, but it is no longer true. I believe this shouldn't be too
  hard to change, but the patch was big enough as it is.
  
I have tested this patch on both the clang test suite, and by running the static analyzer over Postgresql and a large Apple-internal project (mix of Objective-C and C).  

llvm-svn: 55839

clang/Driver/RewriteObjC.cpp

@@ -1978,7 +1978,7 @@ QualType RewriteObjC::getSuperStructType() {
       FieldDecls[i] = FieldDecl::Create(*Context, SourceLocation(), 0, 
                                         FieldTypes[i]);
   
-    SuperStructDecl->defineBody(FieldDecls, 4);
+    SuperStructDecl->defineBody(*Context, FieldDecls, 4);
   }
   return Context->getTagDeclType(SuperStructDecl);
 }
@@ -2005,7 +2005,7 @@ QualType RewriteObjC::getConstantStringStructType() {
       FieldDecls[i] = FieldDecl::Create(*Context, SourceLocation(), 0,
                                         FieldTypes[i]);
   
-    ConstantStringDecl->defineBody(FieldDecls, 4);
+    ConstantStringDecl->defineBody(*Context, FieldDecls, 4);
   }
   return Context->getTagDeclType(ConstantStringDecl);
 }

clang/include/clang/AST/ASTContext.h

@@ -213,7 +213,7 @@ public:
 
   /// getTypeDeclType - Return the unique reference to the type for
   /// the specified type declaration.
-  QualType getTypeDeclType(TypeDecl *Decl);
+  QualType getTypeDeclType(TypeDecl *Decl, TypeDecl* PrevDecl=0);
 
   /// getTypedefType - Return the unique reference to the type for the
   /// specified typename decl.
@@ -467,6 +467,12 @@ private:
   
   void InitBuiltinTypes();
   void InitBuiltinType(QualType &R, BuiltinType::Kind K);
+  
+  /// setRecordDefinition - Used by RecordDecl::defineBody to inform ASTContext
+  ///  about which RecordDecl serves as the definition of a particular
+  ///  struct/union/class.  This will eventually be used by enums as well.
+  void setTagDefinition(TagDecl* R);
+  friend class RecordDecl;
 };
   
 }  // end namespace clang

clang/include/clang/AST/Decl.h

@@ -690,6 +690,15 @@ public:
     return IsDefinition;
   }
   
+  /// getDefinition - Returns the TagDecl that actually defines this 
+  ///  struct/union/class/enum.  When determining whether or not a
+  ///  struct/union/class/enum is completely defined, one should use this method
+  ///  as opposed to 'isDefinition'.  'isDefinition' indicates whether or not a
+  ///  specific TagDecl is defining declaration, not whether or not the
+  ///  struct/union/class/enum type is defined.  This method returns NULL if
+  ///  there is no TagDecl that defines the struct/union/class/enum.
+  TagDecl* getDefinition(ASTContext& C) const;
+  
   const char *getKindName() const {
     switch (getTagKind()) {
     default: assert(0 && "Unknown TagKind!");
@@ -806,13 +815,25 @@ protected:
 
 public:
   static RecordDecl *Create(ASTContext &C, TagKind TK, DeclContext *DC,
-                            SourceLocation L, IdentifierInfo *Id);
+                            SourceLocation L, IdentifierInfo *Id,
+                            RecordDecl* PrevDecl = 0);
 
   virtual void Destroy(ASTContext& C);
       
   bool hasFlexibleArrayMember() const { return HasFlexibleArrayMember; }
   void setHasFlexibleArrayMember(bool V) { HasFlexibleArrayMember = V; }
   
+  /// getDefinition - Returns the RecordDecl that actually defines this 
+  ///  struct/union/class.  When determining whether or not a struct/union/class
+  ///  is completely defined, one should use this method as opposed to
+  ///  'isDefinition'.  'isDefinition' indicates whether or not a specific
+  ///  RecordDecl is defining declaration, not whether or not the record
+  ///  type is defined.  This method returns NULL if there is no RecordDecl
+  ///  that defines the struct/union/tag.
+  RecordDecl* getDefinition(ASTContext& C) const {
+    return cast_or_null<RecordDecl>(TagDecl::getDefinition(C));
+  }
+  
   /// getNumMembers - Return the number of members, or -1 if this is a forward
   /// definition.
   int getNumMembers() const { return NumMembers; }
@@ -844,7 +865,7 @@ public:
   /// defineBody - When created, RecordDecl's correspond to a forward declared
   /// record.  This method is used to mark the decl as being defined, with the
   /// specified contents.
-  void defineBody(FieldDecl **Members, unsigned numMembers);
+  void defineBody(ASTContext& C, FieldDecl **Members, unsigned numMembers);
 
   /// getMember - If the member doesn't exist, or there are no members, this 
   /// function will return 0;

clang/include/clang/AST/DeclCXX.h

@@ -49,7 +49,8 @@ protected:
   }
 public:
   static CXXRecordDecl *Create(ASTContext &C, TagKind TK, DeclContext *DC,
-                               SourceLocation L, IdentifierInfo *Id);
+                               SourceLocation L, IdentifierInfo *Id,
+                               CXXRecordDecl* PrevDecl=0);
   
   const CXXFieldDecl *getMember(unsigned i) const {
     return cast<const CXXFieldDecl>(RecordDecl::getMember(i));

clang/include/clang/AST/Type.h

@@ -1070,6 +1070,7 @@ public:
 
 class TagType : public Type {
   TagDecl *decl;
+  friend class ASTContext;
 
 protected:
   TagType(TagDecl *D, QualType can) : Type(Tagged, can), decl(D) {}

clang/lib/AST/ASTContext.cpp

@@ -483,7 +483,8 @@ ASTContext::getASTObjCInterfaceLayout(const ObjCInterfaceDecl *D) {
 /// specified record (struct/union/class), which indicates its size and field
 /// position information.
 const ASTRecordLayout &ASTContext::getASTRecordLayout(const RecordDecl *D) {
-  assert(D->isDefinition() && "Cannot get layout of forward declarations!");
+  D = D->getDefinition(*this);
+  assert(D && "Cannot get layout of forward declarations!");
 
   // Look up this layout, if already laid out, return what we have.
   const ASTRecordLayout *&Entry = ASTRecordLayouts[D];
@@ -898,7 +899,7 @@ QualType ASTContext::getFunctionType(QualType ResultTy, const QualType *ArgArray
 
 /// getTypeDeclType - Return the unique reference to the type for the
 /// specified type declaration.
-QualType ASTContext::getTypeDeclType(TypeDecl *Decl) {
+QualType ASTContext::getTypeDeclType(TypeDecl *Decl, TypeDecl* PrevDecl) {
   if (Decl->TypeForDecl) return QualType(Decl->TypeForDecl, 0);
   
   if (TypedefDecl *Typedef = dyn_cast_or_null<TypedefDecl>(Decl))
@@ -907,19 +908,31 @@ QualType ASTContext::getTypeDeclType(TypeDecl *Decl) {
              = dyn_cast_or_null<ObjCInterfaceDecl>(Decl))
     return getObjCInterfaceType(ObjCInterface);
 
-  if (CXXRecordDecl *CXXRecord = dyn_cast_or_null<CXXRecordDecl>(Decl))
+  if (CXXRecordDecl *CXXRecord = dyn_cast_or_null<CXXRecordDecl>(Decl)) {
-    Decl->TypeForDecl = new CXXRecordType(CXXRecord);
+    Decl->TypeForDecl = PrevDecl ? PrevDecl->TypeForDecl
-  else if (RecordDecl *Record = dyn_cast_or_null<RecordDecl>(Decl))
+                                 : new CXXRecordType(CXXRecord);
-    Decl->TypeForDecl = new RecordType(Record);
+  }
+  else if (RecordDecl *Record = dyn_cast_or_null<RecordDecl>(Decl)) {
+    Decl->TypeForDecl = PrevDecl ? PrevDecl->TypeForDecl
+                                 : new RecordType(Record);
+  }
   else if (EnumDecl *Enum = dyn_cast_or_null<EnumDecl>(Decl))
     Decl->TypeForDecl = new EnumType(Enum);
   else
     assert(false && "TypeDecl without a type?");
 
-  Types.push_back(Decl->TypeForDecl);
+  if (!PrevDecl) Types.push_back(Decl->TypeForDecl);
   return QualType(Decl->TypeForDecl, 0);
 }
 
+/// setTagDefinition - Used by RecordDecl::defineBody to inform ASTContext
+///  about which RecordDecl serves as the definition of a particular
+///  struct/union/class.  This will eventually be used by enums as well.
+void ASTContext::setTagDefinition(TagDecl* D) {
+  assert (D->isDefinition());
+  cast<TagType>(D->TypeForDecl)->decl = D;  
+}
+
 /// getTypedefType - Return the unique reference to the type for the
 /// specified typename decl.
 QualType ASTContext::getTypedefType(TypedefDecl *Decl) {
@@ -1366,7 +1379,7 @@ QualType ASTContext::getCFConstantStringType() {
       FieldDecls[i] = FieldDecl::Create(*this, SourceLocation(), 0,
                                         FieldTypes[i]);
   
-    CFConstantStringTypeDecl->defineBody(FieldDecls, 4);
+    CFConstantStringTypeDecl->defineBody(*this, FieldDecls, 4);
   }
   
   return getTagDeclType(CFConstantStringTypeDecl);
@@ -1392,7 +1405,7 @@ QualType ASTContext::getObjCFastEnumerationStateType()
       RecordDecl::Create(*this, TagDecl::TK_struct, TUDecl, SourceLocation(),
                          &Idents.get("__objcFastEnumerationState"));
     
-    ObjCFastEnumerationStateTypeDecl->defineBody(FieldDecls, 4);
+    ObjCFastEnumerationStateTypeDecl->defineBody(*this, FieldDecls, 4);
   }
   
   return getTagDeclType(ObjCFastEnumerationStateTypeDecl);

clang/lib/AST/Decl.cpp

@@ -199,6 +199,16 @@ unsigned FunctionDecl::getMinRequiredArguments() const {
   return NumRequiredArgs;
 }
 
+//===----------------------------------------------------------------------===//
+// TagdDecl Implementation
+//===----------------------------------------------------------------------===//
+
+TagDecl* TagDecl::getDefinition(ASTContext& C) const {
+  QualType T = C.getTypeDeclType(const_cast<TagDecl*>(this));
+  TagDecl* D = cast<TagDecl>(cast<TagType>(T)->getDecl());  
+  return D->isDefinition() ? D : 0;
+}
+
 //===----------------------------------------------------------------------===//
 // RecordDecl Implementation
 //===----------------------------------------------------------------------===//
@@ -214,7 +224,8 @@ RecordDecl::RecordDecl(Kind DK, DeclContext *DC, SourceLocation L,
 }
 
 RecordDecl *RecordDecl::Create(ASTContext &C, TagKind TK, DeclContext *DC,
-                               SourceLocation L, IdentifierInfo *Id) {
+                               SourceLocation L, IdentifierInfo *Id,
+                               RecordDecl* PrevDecl) {
   
   void *Mem = C.getAllocator().Allocate<RecordDecl>();
   Kind DK;
@@ -225,7 +236,10 @@ RecordDecl *RecordDecl::Create(ASTContext &C, TagKind TK, DeclContext *DC,
     case TK_union:  DK = Union;  break;
     case TK_class:  DK = Class;  break;
   }
-  return new (Mem) RecordDecl(DK, DC, L, Id);
+  
+  RecordDecl* R = new (Mem) RecordDecl(DK, DC, L, Id);
+  C.getTypeDeclType(R, PrevDecl);
+  return R;
 }
 
 RecordDecl::~RecordDecl() {
@@ -243,7 +257,8 @@ void RecordDecl::Destroy(ASTContext& C) {
 /// defineBody - When created, RecordDecl's correspond to a forward declared
 /// record.  This method is used to mark the decl as being defined, with the
 /// specified contents.
-void RecordDecl::defineBody(FieldDecl **members, unsigned numMembers) {
+void RecordDecl::defineBody(ASTContext& C, FieldDecl **members,
+                            unsigned numMembers) {
   assert(!isDefinition() && "Cannot redefine record!");
   setDefinition(true);
   NumMembers = numMembers;
@@ -251,8 +266,12 @@ void RecordDecl::defineBody(FieldDecl **members, unsigned numMembers) {
     Members = new FieldDecl*[numMembers];
     memcpy(Members, members, numMembers*sizeof(Decl*));
   }
+  
+  // Let ASTContext know that this is the defining RecordDecl this type.
+  C.setTagDefinition(this);
 }
 
+
 FieldDecl *RecordDecl::getMember(IdentifierInfo *II) {
   if (Members == 0 || NumMembers < 0)
     return 0;

clang/lib/AST/DeclCXX.cpp

@@ -1,65 +1,68 @@
-//===--- DeclCXX.cpp - C++ Declaration AST Node Implementation ------------===//
+//===--- DeclCXX.cpp - C++ Declaration AST Node Implementation ------------===//
-//
+//
-//                     The LLVM Compiler Infrastructure
+//                     The LLVM Compiler Infrastructure
-//
+//
-// This file is distributed under the University of Illinois Open Source
+// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// License. See LICENSE.TXT for details.
-//
+//
-//===----------------------------------------------------------------------===//
+//===----------------------------------------------------------------------===//
-//
+//
-// This file implements the C++ related Decl classes.
+// This file implements the C++ related Decl classes.
-//
+//
-//===----------------------------------------------------------------------===//
+//===----------------------------------------------------------------------===//
-
+
-#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclCXX.h"
-#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTContext.h"
-using namespace clang;
+using namespace clang;
-
+
-//===----------------------------------------------------------------------===//
+//===----------------------------------------------------------------------===//
-// Decl Allocation/Deallocation Method Implementations
+// Decl Allocation/Deallocation Method Implementations
-//===----------------------------------------------------------------------===//
+//===----------------------------------------------------------------------===//
- 
+ 
-CXXFieldDecl *CXXFieldDecl::Create(ASTContext &C, CXXRecordDecl *RD,
+CXXFieldDecl *CXXFieldDecl::Create(ASTContext &C, CXXRecordDecl *RD,
-                                   SourceLocation L, IdentifierInfo *Id,
+                                   SourceLocation L, IdentifierInfo *Id,
-                                   QualType T, Expr *BW) {
+                                   QualType T, Expr *BW) {
-  void *Mem = C.getAllocator().Allocate<CXXFieldDecl>();
+  void *Mem = C.getAllocator().Allocate<CXXFieldDecl>();
-  return new (Mem) CXXFieldDecl(RD, L, Id, T, BW);
+  return new (Mem) CXXFieldDecl(RD, L, Id, T, BW);
-}
+}
-
+
-CXXRecordDecl *CXXRecordDecl::Create(ASTContext &C, TagKind TK, DeclContext *DC,
+CXXRecordDecl *CXXRecordDecl::Create(ASTContext &C, TagKind TK, DeclContext *DC,
-                                     SourceLocation L, IdentifierInfo *Id) {
+                                     SourceLocation L, IdentifierInfo *Id,
-  Kind DK;
+                                     CXXRecordDecl* PrevDecl) {
-  switch (TK) {
+  Kind DK;
-    default: assert(0 && "Invalid TagKind!");
+  switch (TK) {
-    case TK_enum:   assert(0 && "Enum TagKind passed for Record!");
+    default: assert(0 && "Invalid TagKind!");
-    case TK_struct: DK = CXXStruct; break;
+    case TK_enum:   assert(0 && "Enum TagKind passed for Record!");
-    case TK_union:  DK = CXXUnion;  break;
+    case TK_struct: DK = CXXStruct; break;
-    case TK_class:  DK = CXXClass;  break;
+    case TK_union:  DK = CXXUnion;  break;
-  }
+    case TK_class:  DK = CXXClass;  break;
-  void *Mem = C.getAllocator().Allocate<CXXRecordDecl>();
+  }
-  return new (Mem) CXXRecordDecl(DK, DC, L, Id);
+  void *Mem = C.getAllocator().Allocate<CXXRecordDecl>();
-}
+  CXXRecordDecl* R = new (Mem) CXXRecordDecl(DK, DC, L, Id);
-
+  C.getTypeDeclType(R, PrevDecl);  
-CXXMethodDecl *
+  return R;
-CXXMethodDecl::Create(ASTContext &C, CXXRecordDecl *RD,
+}
-                      SourceLocation L, IdentifierInfo *Id,
+
-                      QualType T, bool isStatic, bool isInline,
+CXXMethodDecl *
-                      ScopedDecl *PrevDecl) {
+CXXMethodDecl::Create(ASTContext &C, CXXRecordDecl *RD,
-  void *Mem = C.getAllocator().Allocate<CXXMethodDecl>();
+                      SourceLocation L, IdentifierInfo *Id,
-  return new (Mem) CXXMethodDecl(RD, L, Id, T, isStatic, isInline, PrevDecl);
+                      QualType T, bool isStatic, bool isInline,
-}
+                      ScopedDecl *PrevDecl) {
-
+  void *Mem = C.getAllocator().Allocate<CXXMethodDecl>();
-QualType CXXMethodDecl::getThisType(ASTContext &C) const {
+  return new (Mem) CXXMethodDecl(RD, L, Id, T, isStatic, isInline, PrevDecl);
-  assert(isInstance() && "No 'this' for static methods!");
+}
-  QualType ClassTy = C.getTagDeclType(cast<CXXRecordDecl>(getParent()));
+
-  QualType ThisTy = C.getPointerType(ClassTy);
+QualType CXXMethodDecl::getThisType(ASTContext &C) const {
-  ThisTy.addConst();
+  assert(isInstance() && "No 'this' for static methods!");
-  return ThisTy;
+  QualType ClassTy = C.getTagDeclType(cast<CXXRecordDecl>(getParent()));
-}
+  QualType ThisTy = C.getPointerType(ClassTy);
-
+  ThisTy.addConst();
-CXXClassVarDecl *CXXClassVarDecl::Create(ASTContext &C, CXXRecordDecl *RD,
+  return ThisTy;
-                                   SourceLocation L, IdentifierInfo *Id,
+}
-                                   QualType T, ScopedDecl *PrevDecl) {
+
-  void *Mem = C.getAllocator().Allocate<CXXClassVarDecl>();
+CXXClassVarDecl *CXXClassVarDecl::Create(ASTContext &C, CXXRecordDecl *RD,
-  return new (Mem) CXXClassVarDecl(RD, L, Id, T, PrevDecl);
+                                   SourceLocation L, IdentifierInfo *Id,
-}
+                                   QualType T, ScopedDecl *PrevDecl) {
+  void *Mem = C.getAllocator().Allocate<CXXClassVarDecl>();
+  return new (Mem) CXXClassVarDecl(RD, L, Id, T, PrevDecl);
+}

clang/lib/CodeGen/CGObjCMac.cpp

@@ -1872,7 +1872,7 @@ ObjCTypesHelper::ObjCTypesHelper(CodeGen::CodeGenModule &cgm)
                                     Ctx.getObjCIdType());
   FieldDecls[1] = FieldDecl::Create(Ctx, SourceLocation(), 0,
                                     Ctx.getObjCClassType());
-  RD->defineBody(FieldDecls, 2);
+  RD->defineBody(Ctx, FieldDecls, 2);
 
   SuperCTy = Ctx.getTagDeclType(RD);
   SuperPtrCTy = Ctx.getPointerType(SuperCTy);

clang/lib/Sema/Sema.h

@@ -251,6 +251,11 @@ private:
   virtual DeclTy *ActOnTag(Scope *S, unsigned TagType, TagKind TK,
                            SourceLocation KWLoc, IdentifierInfo *Name,
                            SourceLocation NameLoc, AttributeList *Attr);
+  
+  DeclTy* ActOnTagStruct(Scope *S, TagDecl::TagKind Kind, TagKind TK,
+                         SourceLocation KWLoc, IdentifierInfo *Name,
+                         SourceLocation NameLoc, AttributeList *Attr);  
+  
   virtual void ActOnDefs(Scope *S, SourceLocation DeclStart,
                          IdentifierInfo *ClassName,
                          llvm::SmallVectorImpl<DeclTy*> &Decls);

clang/lib/Sema/SemaDecl.cpp

@@ -1682,6 +1682,12 @@ Sema::DeclTy *Sema::ActOnTag(Scope *S, unsigned TagType, TagKind TK,
   case DeclSpec::TST_enum:   Kind = TagDecl::TK_enum; break;
   }
   
+  // Two code paths: a new one for structs/unions/classes where we create
+  //   separate decls for forward declarations, and an old (eventually to
+  //   be removed) code path for enums.
+  if (Kind != TagDecl::TK_enum)
+    return ActOnTagStruct(S, Kind, TK, KWLoc, Name, NameLoc, Attr);
+  
   // If this is a named struct, check to see if there was a previous forward
   // declaration or definition.
   // Use ScopedDecl instead of TagDecl, because a NamespaceDecl may come up.
@@ -1781,6 +1787,121 @@ Sema::DeclTy *Sema::ActOnTag(Scope *S, unsigned TagType, TagKind TK,
   return New;
 }
 
+/// ActOnTagStruct - New "ActOnTag" logic for structs/unions/classes.  Unlike
+///  the logic for enums, we create separate decls for forward declarations.
+///  This is called by ActOnTag, but eventually will replace its logic.
+Sema::DeclTy *Sema::ActOnTagStruct(Scope *S, TagDecl::TagKind Kind, TagKind TK,
+                             SourceLocation KWLoc, IdentifierInfo *Name,
+                             SourceLocation NameLoc, AttributeList *Attr) {
+  
+  // If this is a named struct, check to see if there was a previous forward
+  // declaration or definition.
+  // Use ScopedDecl instead of TagDecl, because a NamespaceDecl may come up.
+  ScopedDecl *PrevDecl = 
+    dyn_cast_or_null<ScopedDecl>(LookupDecl(Name, Decl::IDNS_Tag, S));
+  
+  if (PrevDecl) {    
+    assert((isa<TagDecl>(PrevDecl) || isa<NamespaceDecl>(PrevDecl)) &&
+           "unexpected Decl type");
+    
+    if (TagDecl *PrevTagDecl = dyn_cast<TagDecl>(PrevDecl)) {
+      // If this is a use of a previous tag, or if the tag is already declared
+      // in the same scope (so that the definition/declaration completes or
+      // rementions the tag), reuse the decl.
+      if (TK == TK_Reference ||
+          IdResolver.isDeclInScope(PrevDecl, CurContext, S)) {
+        // Make sure that this wasn't declared as an enum and now used as a
+        // struct or something similar.
+        if (PrevTagDecl->getTagKind() != Kind) {
+          Diag(KWLoc, diag::err_use_with_wrong_tag, Name->getName());
+          Diag(PrevDecl->getLocation(), diag::err_previous_use);
+          // Recover by making this an anonymous redefinition.
+          Name = 0;
+          PrevDecl = 0;
+        } else {
+          // If this is a use, return the original decl.
+          
+          // FIXME: In the future, return a variant or some other clue
+          //  for the consumer of this Decl to know it doesn't own it.
+          //  For our current ASTs this shouldn't be a problem, but will
+          //  need to be changed with DeclGroups.
+          if (TK == TK_Reference)
+            return PrevDecl;
+          
+          // The new decl is a definition?
+          if (TK == TK_Definition) {
+            // Diagnose attempts to redefine a tag.
+            if (RecordDecl* DefRecord =
+                cast<RecordDecl>(PrevTagDecl)->getDefinition(Context)) {
+              Diag(NameLoc, diag::err_redefinition, Name->getName());
+              Diag(DefRecord->getLocation(), diag::err_previous_definition);
+              // If this is a redefinition, recover by making this struct be
+              // anonymous, which will make any later references get the previous
+              // definition.
+              Name = 0;
+              PrevDecl = 0;
+            }
+            // Okay, this is definition of a previously declared or referenced
+            // tag.  We're going to create a new Decl.
+          }
+        }
+        // If we get here we have (another) forward declaration.  Just create
+        // a new decl.        
+      }
+      else {
+        // If we get here, this is a definition of a new struct type in a nested
+        // scope, e.g. "struct foo; void bar() { struct foo; }", just create a 
+        // new decl/type.  We set PrevDecl to NULL so that the Records
+        // have distinct types.
+        PrevDecl = 0;
+      }
+    } else {
+      // PrevDecl is a namespace.
+      if (IdResolver.isDeclInScope(PrevDecl, CurContext, S)) {
+        // The tag name clashes with a namespace name, issue an error and
+        // recover by making this tag be anonymous.
+        Diag(NameLoc, diag::err_redefinition_different_kind, Name->getName());
+        Diag(PrevDecl->getLocation(), diag::err_previous_definition);
+        Name = 0;
+      }
+    }
+  }
+  
+  // If there is an identifier, use the location of the identifier as the
+  // location of the decl, otherwise use the location of the struct/union
+  // keyword.
+  SourceLocation Loc = NameLoc.isValid() ? NameLoc : KWLoc;
+  
+  // Otherwise, if this is the first time we've seen this tag, create the decl.
+  TagDecl *New;
+    
+  // FIXME: Tag decls should be chained to any simultaneous vardecls, e.g.:
+  // struct X { int A; } D;    D should chain to X.
+  if (getLangOptions().CPlusPlus)
+    New = CXXRecordDecl::Create(Context, Kind, CurContext, Loc, Name,
+                                dyn_cast_or_null<CXXRecordDecl>(PrevDecl));
+  else
+    New = RecordDecl::Create(Context, Kind, CurContext, Loc, Name,
+                             dyn_cast_or_null<RecordDecl>(PrevDecl));
+  
+  // If this has an identifier, add it to the scope stack.
+  if ((TK == TK_Definition || !PrevDecl) && Name) {
+    // The scope passed in may not be a decl scope.  Zip up the scope tree until
+    // we find one that is.
+    while ((S->getFlags() & Scope::DeclScope) == 0)
+      S = S->getParent();
+    
+    // Add it to the decl chain.
+    PushOnScopeChains(New, S);
+  }
+  
+  if (Attr)
+    ProcessDeclAttributeList(New, Attr);
+
+  return New;
+}
+
+
 /// Collect the instance variables declared in an Objective-C object.  Used in
 /// the creation of structures from objects using the @defs directive.
 static void CollectIvars(ObjCInterfaceDecl *Class, ASTContext& Ctx,
@@ -1977,17 +2098,19 @@ void Sema::ActOnFields(Scope* S,
   assert(EnclosingDecl && "missing record or interface decl");
   RecordDecl *Record = dyn_cast<RecordDecl>(EnclosingDecl);
   
-  if (Record && Record->isDefinition()) {
+  if (Record)
-    // Diagnose code like:
+    if (RecordDecl* DefRecord = Record->getDefinition(Context)) {
-    //     struct S { struct S {} X; };
+      // Diagnose code like:
-    // We discover this when we complete the outer S.  Reject and ignore the
+      //     struct S { struct S {} X; };
-    // outer S.
+      // We discover this when we complete the outer S.  Reject and ignore the
-    Diag(Record->getLocation(), diag::err_nested_redefinition,
+      // outer S.
-         Record->getKindName());
+      Diag(DefRecord->getLocation(), diag::err_nested_redefinition,
-    Diag(RecLoc, diag::err_previous_definition);
+           DefRecord->getKindName());
-    Record->setInvalidDecl();
+      Diag(RecLoc, diag::err_previous_definition);
-    return;
+      Record->setInvalidDecl();
-  }
+      return;
+    }
+  
   // Verify that all the fields are okay.
   unsigned NumNamedMembers = 0;
   llvm::SmallVector<FieldDecl*, 32> RecFields;
@@ -2099,7 +2222,7 @@ void Sema::ActOnFields(Scope* S,
  
   // Okay, we successfully defined 'Record'.
   if (Record) {
-    Record->defineBody(&RecFields[0], RecFields.size());
+    Record->defineBody(Context, &RecFields[0], RecFields.size());
     // If this is a C++ record, HandleTagDeclDefinition will be invoked in
     // Sema::ActOnFinishCXXClassDef.
     if (!isa<CXXRecordDecl>(Record))