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


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

1 files changed, 135 insertions, 12 deletions

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 467da81305..df91182383 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/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()) {
-    // Diagnose code like:
-    //     struct S { struct S {} X; };
-    // We discover this when we complete the outer S.  Reject and ignore the
-    // outer S.
-    Diag(Record->getLocation(), diag::err_nested_redefinition,
-         Record->getKindName());
-    Diag(RecLoc, diag::err_previous_definition);
-    Record->setInvalidDecl();
-    return;
-  }
+  if (Record)
+    if (RecordDecl* DefRecord = Record->getDefinition(Context)) {
+      // Diagnose code like:
+      //     struct S { struct S {} X; };
+      // We discover this when we complete the outer S.  Reject and ignore the
+      // outer S.
+      Diag(DefRecord->getLocation(), diag::err_nested_redefinition,
+           DefRecord->getKindName());
+      Diag(RecLoc, diag::err_previous_definition);
+      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))