Wire up redeclaration chains for Objective-C protocols, so that both
forward declarations and definitions of an Objective-C protocol are
represented within a single chain of ObjCProtocolDecls.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@147412 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/AST/DeclObjC.h b/include/clang/AST/DeclObjC.h
index e5c89b8..f99203a 100644
--- a/include/clang/AST/DeclObjC.h
+++ b/include/clang/AST/DeclObjC.h
@@ -1089,12 +1089,8 @@
   
   ObjCProtocolDecl(DeclContext *DC, IdentifierInfo *Id,
                    SourceLocation nameLoc, SourceLocation atStartLoc,
-                   bool isForwardDecl)
-    : ObjCContainerDecl(ObjCProtocol, DC, Id, nameLoc, atStartLoc),
-      Data(0),
-      InitiallyForwardDecl(isForwardDecl),
-      isForwardProtoDecl(isForwardDecl) {
-  }
+                   ObjCProtocolDecl *PrevDecl,
+                   bool isForwardDecl);
 
   void allocateDefinitionData();
 
@@ -1108,6 +1104,7 @@
                                   IdentifierInfo *Id,
                                   SourceLocation nameLoc,
                                   SourceLocation atStartLoc,
+                                  ObjCProtocolDecl *PrevDecl,
                                   bool isForwardDecl);
 
   const ObjCProtocolList &getReferencedProtocols() const {
@@ -1194,8 +1191,6 @@
   /// false when we see the definition, but this will remain true.
   bool isInitiallyForwardDecl() const { return InitiallyForwardDecl; }
 
-  void completedForwardDecl();
-
   // Location information, modeled after the Stmt API.
   SourceLocation getLocStart() const { return getAtStartLoc(); } // '@'protocol
   SourceLocation getLocEnd() const { return EndLoc; }
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index d9f9ab5..0ea55f8 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -1327,10 +1327,11 @@
   TypedefDecl *ParseTypedefDecl(Scope *S, Declarator &D, QualType T,
                                 TypeSourceInfo *TInfo);
   bool isIncompatibleTypedef(TypeDecl *Old, TypedefNameDecl *New);
+  void mergeDeclAttributes(Decl *New, Decl *Old, bool MergeDeprecation = true);
   void MergeTypedefNameDecl(TypedefNameDecl *New, LookupResult &OldDecls);
   bool MergeFunctionDecl(FunctionDecl *New, Decl *Old);
   bool MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old);
-  void mergeObjCMethodDecls(ObjCMethodDecl *New, const ObjCMethodDecl *Old);
+  void mergeObjCMethodDecls(ObjCMethodDecl *New, ObjCMethodDecl *Old);
   void MergeVarDecl(VarDecl *New, LookupResult &OldDecls);
   void MergeVarDeclTypes(VarDecl *New, VarDecl *Old);
   void MergeVarDeclExceptionSpecs(VarDecl *New, VarDecl *Old);
@@ -1755,7 +1756,9 @@
   bool LookupParsedName(LookupResult &R, Scope *S, CXXScopeSpec *SS,
                         bool AllowBuiltinCreation = false,
                         bool EnteringContext = false);
-  ObjCProtocolDecl *LookupProtocol(IdentifierInfo *II, SourceLocation IdLoc);
+  ObjCProtocolDecl *LookupProtocol(IdentifierInfo *II, SourceLocation IdLoc,
+                                   RedeclarationKind Redecl
+                                     = NotForRedeclaration);
 
   void LookupOverloadedOperatorName(OverloadedOperatorKind Op, Scope *S,
                                     QualType T1, QualType T2,
diff --git a/lib/AST/ASTImporter.cpp b/lib/AST/ASTImporter.cpp
index fe70494..9931961 100644
--- a/lib/AST/ASTImporter.cpp
+++ b/lib/AST/ASTImporter.cpp
@@ -3124,11 +3124,10 @@
       ToProto = ObjCProtocolDecl::Create(Importer.getToContext(), DC,
                                          Name.getAsIdentifierInfo(), Loc,
                                          Importer.Import(D->getAtStartLoc()),
+                                         /*PrevDecl=*/0,
                                          D->isInitiallyForwardDecl());
       ToProto->setLexicalDeclContext(LexicalDC);
       LexicalDC->addDeclInternal(ToProto);
-      if (D->isInitiallyForwardDecl() && D->hasDefinition())
-        ToProto->completedForwardDecl();
     }
     if (!ToProto->hasDefinition())
       ToProto->startDefinition();
diff --git a/lib/AST/DeclObjC.cpp b/lib/AST/DeclObjC.cpp
index 341f0f5..27da21a 100644
--- a/lib/AST/DeclObjC.cpp
+++ b/lib/AST/DeclObjC.cpp
@@ -967,12 +967,32 @@
 
 void ObjCProtocolDecl::anchor() { }
 
+ObjCProtocolDecl::ObjCProtocolDecl(DeclContext *DC, IdentifierInfo *Id,
+                                   SourceLocation nameLoc, 
+                                   SourceLocation atStartLoc,
+                                   ObjCProtocolDecl *PrevDecl,
+                                   bool isForwardDecl)
+  : ObjCContainerDecl(ObjCProtocol, DC, Id, nameLoc, atStartLoc),
+    Data(0),
+    InitiallyForwardDecl(isForwardDecl),
+    isForwardProtoDecl(isForwardDecl) 
+{
+  setPreviousDeclaration(PrevDecl);
+  if (PrevDecl)
+    Data = PrevDecl->Data;
+}
+
 ObjCProtocolDecl *ObjCProtocolDecl::Create(ASTContext &C, DeclContext *DC,
                                            IdentifierInfo *Id,
                                            SourceLocation nameLoc,
                                            SourceLocation atStartLoc,
+                                           ObjCProtocolDecl *PrevDecl,
                                            bool isForwardDecl) {
-  return new (C) ObjCProtocolDecl(DC, Id, nameLoc, atStartLoc, isForwardDecl);
+  ObjCProtocolDecl *Result 
+    = new (C) ObjCProtocolDecl(DC, Id, nameLoc, atStartLoc, PrevDecl,
+                               isForwardDecl);
+  
+  return Result;
 }
 
 ObjCProtocolDecl *ObjCProtocolDecl::lookupProtocolNamed(IdentifierInfo *Name) {
@@ -1016,12 +1036,7 @@
   for (redecl_iterator RD = redecls_begin(), RDEnd = redecls_end();
        RD != RDEnd; ++RD)
     RD->Data = this->Data;
-}
-
-void ObjCProtocolDecl::completedForwardDecl() {
-  assert(!hasDefinition() && "Only valid to call for forward refs");
-  isForwardProtoDecl = false;
-  startDefinition();
+  
   if (ASTMutationListener *L = getASTContext().getASTMutationListener())
     L->CompletedObjCForwardRef(this);
 }
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index c8292b9..a1ccf94 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -1540,36 +1540,37 @@
 }
 
 /// mergeDeclAttributes - Copy attributes from the Old decl to the New one.
-static void mergeDeclAttributes(Decl *newDecl, const Decl *oldDecl,
-                                ASTContext &C, bool mergeDeprecation = true) {
-  if (!oldDecl->hasAttrs())
+void Sema::mergeDeclAttributes(Decl *New, Decl *Old,
+                               bool MergeDeprecation) {
+  if (!Old->hasAttrs())
     return;
 
-  bool foundAny = newDecl->hasAttrs();
+  bool foundAny = New->hasAttrs();
 
   // Ensure that any moving of objects within the allocated map is done before
   // we process them.
-  if (!foundAny) newDecl->setAttrs(AttrVec());
+  if (!foundAny) New->setAttrs(AttrVec());
 
   for (specific_attr_iterator<InheritableAttr>
-       i = oldDecl->specific_attr_begin<InheritableAttr>(),
-       e = oldDecl->specific_attr_end<InheritableAttr>(); i != e; ++i) {
+         i = Old->specific_attr_begin<InheritableAttr>(),
+         e = Old->specific_attr_end<InheritableAttr>(); 
+       i != e; ++i) {
     // Ignore deprecated/unavailable/availability attributes if requested.
-    if (!mergeDeprecation &&
+    if (!MergeDeprecation &&
         (isa<DeprecatedAttr>(*i) || 
          isa<UnavailableAttr>(*i) ||
          isa<AvailabilityAttr>(*i)))
       continue;
 
-    if (!DeclHasAttr(newDecl, *i)) {
-      InheritableAttr *newAttr = cast<InheritableAttr>((*i)->clone(C));
+    if (!DeclHasAttr(New, *i)) {
+      InheritableAttr *newAttr = cast<InheritableAttr>((*i)->clone(Context));
       newAttr->setInherited(true);
-      newDecl->addAttr(newAttr);
+      New->addAttr(newAttr);
       foundAny = true;
     }
   }
 
-  if (!foundAny) newDecl->dropAttrs();
+  if (!foundAny) New->dropAttrs();
 }
 
 /// mergeParamDeclAttributes - Copy attributes from the old parameter
@@ -2035,7 +2036,7 @@
 /// \returns false
 bool Sema::MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old) {
   // Merge the attributes
-  mergeDeclAttributes(New, Old, Context);
+  mergeDeclAttributes(New, Old);
 
   // Merge the storage class.
   if (Old->getStorageClass() != SC_Extern &&
@@ -2061,13 +2062,13 @@
 
 
 void Sema::mergeObjCMethodDecls(ObjCMethodDecl *newMethod,
-                                const ObjCMethodDecl *oldMethod) {
+                                ObjCMethodDecl *oldMethod) {
   // We don't want to merge unavailable and deprecated attributes
   // except from interface to implementation.
   bool mergeDeprecation = isa<ObjCImplDecl>(newMethod->getDeclContext());
 
   // Merge the attributes.
-  mergeDeclAttributes(newMethod, oldMethod, Context, mergeDeprecation);
+  mergeDeclAttributes(newMethod, oldMethod, mergeDeprecation);
 
   // Merge attributes from the parameters.
   ObjCMethodDecl::param_const_iterator oi = oldMethod->param_begin();
@@ -2174,7 +2175,7 @@
     New->setInvalidDecl();
   }
   
-  mergeDeclAttributes(New, Old, Context);
+  mergeDeclAttributes(New, Old);
   // Warn if an already-declared variable is made a weak_import in a subsequent 
   // declaration
   if (New->getAttr<WeakImportAttr>() &&
diff --git a/lib/Sema/SemaDeclObjC.cpp b/lib/Sema/SemaDeclObjC.cpp
index eb17861..c4261ec 100644
--- a/lib/Sema/SemaDeclObjC.cpp
+++ b/lib/Sema/SemaDeclObjC.cpp
@@ -569,46 +569,50 @@
   bool err = false;
   // FIXME: Deal with AttrList.
   assert(ProtocolName && "Missing protocol identifier");
-  ObjCProtocolDecl *PDecl = LookupProtocol(ProtocolName, ProtocolLoc);
-  if (PDecl) {
-    // Protocol already seen. Better be a forward protocol declaration
-    if (ObjCProtocolDecl *Def = PDecl->getDefinition()) {
-      Diag(ProtocolLoc, diag::warn_duplicate_protocol_def) << ProtocolName;
-      Diag(Def->getLocation(), diag::note_previous_definition);
+  ObjCProtocolDecl *PrevDecl = LookupProtocol(ProtocolName, ProtocolLoc,
+                                              ForRedeclaration);
+  ObjCProtocolDecl *PDecl = 0;
+  if (ObjCProtocolDecl *Def = PrevDecl? PrevDecl->getDefinition() : 0) {
+    // If we already have a definition, complain.
+    Diag(ProtocolLoc, diag::warn_duplicate_protocol_def) << ProtocolName;
+    Diag(Def->getLocation(), diag::note_previous_definition);
 
-      // Create a new protocol that is completely distinct from previous
-      // declarations, and do not make this protocol available for name lookup.
-      // That way, we'll end up completely ignoring the duplicate.
-      // FIXME: Can we turn this into an error?
-      PDecl = ObjCProtocolDecl::Create(Context, CurContext, ProtocolName,
-                                       ProtocolLoc, AtProtoInterfaceLoc,
-                                       /*isForwardDecl=*/false);
-      PDecl->startDefinition();
-    } else {
+    // Create a new protocol that is completely distinct from previous
+    // declarations, and do not make this protocol available for name lookup.
+    // That way, we'll end up completely ignoring the duplicate.
+    // FIXME: Can we turn this into an error?
+    PDecl = ObjCProtocolDecl::Create(Context, CurContext, ProtocolName,
+                                     ProtocolLoc, AtProtoInterfaceLoc,
+                                     /*PrevDecl=*/0,
+                                     /*isForwardDecl=*/false);
+    PDecl->startDefinition();
+  } else {
+    if (PrevDecl) {
+      // Check for circular dependencies among protocol declarations. This can
+      // only happen if this protocol was forward-declared.
       ObjCList<ObjCProtocolDecl> PList;
       PList.set((ObjCProtocolDecl *const*)ProtoRefs, NumProtoRefs, Context);
       err = CheckForwardProtocolDeclarationForCircularDependency(
-              ProtocolName, ProtocolLoc, PDecl->getLocation(), PList);
-  
-      // Make sure the cached decl gets a valid start location.
-      PDecl->setAtStartLoc(AtProtoInterfaceLoc);
-      PDecl->setLocation(ProtocolLoc);
-      // Since this ObjCProtocolDecl was created by a forward declaration,
-      // we now add it to the DeclContext since it wasn't added before
-      PDecl->setLexicalDeclContext(CurContext);
-      CurContext->addDecl(PDecl);
-      PDecl->completedForwardDecl();
+              ProtocolName, ProtocolLoc, PrevDecl->getLocation(), PList);
     }
-  } else {
+
+    // Create the new declaration.
     PDecl = ObjCProtocolDecl::Create(Context, CurContext, ProtocolName,
                                      ProtocolLoc, AtProtoInterfaceLoc,
+                                     /*PrevDecl=*/PrevDecl, 
                                      /*isForwardDecl=*/false);
+    
     PushOnScopeChains(PDecl, TUScope);
     PDecl->startDefinition();
   }
   
   if (AttrList)
     ProcessDeclAttributeList(TUScope, PDecl, AttrList);
+  
+  // Merge attributes from previous declarations.
+  if (PrevDecl)
+    mergeDeclAttributes(PDecl, PrevDecl);
+
   if (!err && NumProtoRefs ) {
     /// Check then save referenced protocols.
     PDecl->setProtocolList((ObjCProtocolDecl**)ProtoRefs, NumProtoRefs,
@@ -700,22 +704,26 @@
 
   for (unsigned i = 0; i != NumElts; ++i) {
     IdentifierInfo *Ident = IdentList[i].first;
-    ObjCProtocolDecl *PDecl = LookupProtocol(Ident, IdentList[i].second);
-    bool isNew = false;
-    if (PDecl == 0) { // Not already seen?
-      PDecl = ObjCProtocolDecl::Create(Context, CurContext, Ident,
-                                       IdentList[i].second, AtProtocolLoc,
-                                       /*isForwardDecl=*/true);
-      PushOnScopeChains(PDecl, TUScope, false);
-      isNew = true;
-    }
+    ObjCProtocolDecl *PrevDecl = LookupProtocol(Ident, IdentList[i].second,
+                                                ForRedeclaration);
+    ObjCProtocolDecl *PDecl
+      = ObjCProtocolDecl::Create(Context, CurContext, Ident, 
+                                 IdentList[i].second, AtProtocolLoc,
+                                 PrevDecl, /*isForwardDecl=*/true);
+        
+    PushOnScopeChains(PDecl, TUScope);
+    
     if (attrList) {
       ProcessDeclAttributeList(TUScope, PDecl, attrList);
-      if (!isNew) {
+      if (PrevDecl) {
         if (ASTMutationListener *L = Context.getASTMutationListener())
           L->UpdatedAttributeList(PDecl);
       }
     }
+    
+    if (PrevDecl)
+      mergeDeclAttributes(PDecl, PrevDecl);
+
     Protocols.push_back(PDecl);
     ProtoLocs.push_back(IdentList[i].second);
   }
diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index f7dd69a..63d14f4 100644
--- a/lib/Sema/SemaLookup.cpp
+++ b/lib/Sema/SemaLookup.cpp
@@ -2180,9 +2180,10 @@
 
 /// \brief Find the protocol with the given name, if any.
 ObjCProtocolDecl *Sema::LookupProtocol(IdentifierInfo *II,
-                                       SourceLocation IdLoc) {
+                                       SourceLocation IdLoc,
+                                       RedeclarationKind Redecl) {
   Decl *D = LookupSingleName(TUScope, II, IdLoc,
-                             LookupObjCProtocolName);
+                             LookupObjCProtocolName, Redecl);
   return cast_or_null<ObjCProtocolDecl>(D);
 }
 
diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index 38b6941..566b0e7 100644
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -1950,7 +1950,7 @@
     break;
   case DECL_OBJC_PROTOCOL:
     D = ObjCProtocolDecl::Create(Context, 0, 0, SourceLocation(),
-                                 SourceLocation(), 0);
+                                 SourceLocation(), 0, false);
     break;
   case DECL_OBJC_AT_DEFS_FIELD:
     D = ObjCAtDefsFieldDecl::Create(Context, 0, SourceLocation(),
diff --git a/test/Modules/Inputs/redecl-merge-bottom.h b/test/Modules/Inputs/redecl-merge-bottom.h
index 50c191c..b3cc74b 100644
--- a/test/Modules/Inputs/redecl-merge-bottom.h
+++ b/test/Modules/Inputs/redecl-merge-bottom.h
@@ -8,6 +8,8 @@
 
 @class A;
 
+@protocol P1;
+
 void refers_to_C4(C4*);
 
 #ifdef __cplusplus
diff --git a/test/Modules/Inputs/redecl-merge-left.h b/test/Modules/Inputs/redecl-merge-left.h
index c4789ad..2d77bad 100644
--- a/test/Modules/Inputs/redecl-merge-left.h
+++ b/test/Modules/Inputs/redecl-merge-left.h
@@ -10,6 +10,11 @@
 
 @class A;
 
+@protocol P1;
+@protocol P2
+- (void)protoMethod2;
+@end
+
 // Test declarations in different modules with no common initial
 // declaration.
 @class C;
diff --git a/test/Modules/Inputs/redecl-merge-right.h b/test/Modules/Inputs/redecl-merge-right.h
index f86f110..c5e35b1 100644
--- a/test/Modules/Inputs/redecl-merge-right.h
+++ b/test/Modules/Inputs/redecl-merge-right.h
@@ -9,6 +9,18 @@
 
 @class B;
 
+@protocol P1
+- (void)protoMethod1;
+@end
+
+@protocol P1;
+
+@protocol P2;
+
+@protocol P2;
+
+@protocol P2;
+
 // Test declarations in different modules with no common initial
 // declaration.
 @class C;
diff --git a/test/Modules/Inputs/redecl-merge-top.h b/test/Modules/Inputs/redecl-merge-top.h
index 64e2cc5..64c0c92 100644
--- a/test/Modules/Inputs/redecl-merge-top.h
+++ b/test/Modules/Inputs/redecl-merge-top.h
@@ -6,6 +6,11 @@
 
 @class B;
 
+@protocol P1;
+
+@protocol P2;
+@protocol P2;
+
 #ifdef __cplusplus
 template<typename T> class Vector;
 #endif
diff --git a/test/Modules/redecl-merge.m b/test/Modules/redecl-merge.m
index ea04037..d792739 100644
--- a/test/Modules/redecl-merge.m
+++ b/test/Modules/redecl-merge.m
@@ -26,6 +26,11 @@
 
 @class B;
 
+void testProtoMerge(id<P1> p1, id<P2> p2) {
+  [p1 protoMethod1];
+  [p2 protoMethod2];
+}
+
 // Test redeclarations of entities in explicit submodules, to make
 // sure we're maintaining the declaration chains even when normal name
 // lookup can't see what we're looking for.