Modules: for ObjectiveC try to keep the definition invariant.

When deserializing ObjCInterfaceDecl with definition data, if we already have
a definition, try to keep the definition invariant; also pull in the
categories even if it is not what getDefinition returns (this effectively
combines categories).

rdar://27926200
rdar://26708823

llvm-svn: 281119

22 files changed, 157 insertions, 36 deletions

diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 3cc54048188..70c7b7808c4 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -133,6 +133,10 @@ namespace clang {
                                const RecordData &R, unsigned &I);
     void MergeDefinitionData(CXXRecordDecl *D,
                              struct CXXRecordDecl::DefinitionData &&NewDD);
+    void ReadObjCDefinitionData(struct ObjCInterfaceDecl::DefinitionData &Data,
+                                const RecordData &R, unsigned &I);
+    void MergeDefinitionData(ObjCInterfaceDecl *D,
+                             struct ObjCInterfaceDecl::DefinitionData &&NewDD);
 
     static NamedDecl *getAnonymousDeclForMerging(ASTReader &Reader,
                                                  DeclContext *DC,
@@ -981,6 +985,43 @@ ObjCTypeParamList *ASTDeclReader::ReadObjCTypeParamList() {
                                    typeParams, rAngleLoc);
 }
 
+void ASTDeclReader::ReadObjCDefinitionData(
+         struct ObjCInterfaceDecl::DefinitionData &Data,
+         const RecordData &R, unsigned &I) {
+  // Read the superclass.
+  Data.SuperClassTInfo = GetTypeSourceInfo(Record, Idx);
+
+  Data.EndLoc = ReadSourceLocation(Record, Idx);
+  Data.HasDesignatedInitializers = Record[Idx++];
+   
+  // Read the directly referenced protocols and their SourceLocations.
+  unsigned NumProtocols = Record[Idx++];
+  SmallVector<ObjCProtocolDecl *, 16> Protocols;
+  Protocols.reserve(NumProtocols);
+  for (unsigned I = 0; I != NumProtocols; ++I)
+    Protocols.push_back(ReadDeclAs<ObjCProtocolDecl>(Record, Idx));
+  SmallVector<SourceLocation, 16> ProtoLocs;
+  ProtoLocs.reserve(NumProtocols);
+  for (unsigned I = 0; I != NumProtocols; ++I)
+    ProtoLocs.push_back(ReadSourceLocation(Record, Idx));
+  Data.ReferencedProtocols.set(Protocols.data(), NumProtocols, ProtoLocs.data(),
+                               Reader.getContext());
+ 
+  // Read the transitive closure of protocols referenced by this class.
+  NumProtocols = Record[Idx++];
+  Protocols.clear();
+  Protocols.reserve(NumProtocols);
+  for (unsigned I = 0; I != NumProtocols; ++I)
+    Protocols.push_back(ReadDeclAs<ObjCProtocolDecl>(Record, Idx));
+  Data.AllReferencedProtocols.set(Protocols.data(), NumProtocols,
+                                  Reader.getContext());
+}
+
+void ASTDeclReader::MergeDefinitionData(ObjCInterfaceDecl *D,
+         struct ObjCInterfaceDecl::DefinitionData &&NewDD) {
+  // FIXME: odr checking?
+}
+
 void ASTDeclReader::VisitObjCInterfaceDecl(ObjCInterfaceDecl *ID) {
   RedeclarableResult Redecl = VisitRedeclarable(ID);
   VisitObjCContainerDecl(ID);
@@ -991,43 +1032,22 @@ void ASTDeclReader::VisitObjCInterfaceDecl(ObjCInterfaceDecl *ID) {
   if (Record[Idx++]) {
     // Read the definition.
     ID->allocateDefinitionData();
-    
-    // Set the definition data of the canonical declaration, so other
-    // redeclarations will see it.
-    ID->getCanonicalDecl()->Data = ID->Data;
-    
-    ObjCInterfaceDecl::DefinitionData &Data = ID->data();
-    
-    // Read the superclass.
-    Data.SuperClassTInfo = GetTypeSourceInfo(Record, Idx);
 
-    Data.EndLoc = ReadSourceLocation(Record, Idx);
-    Data.HasDesignatedInitializers = Record[Idx++];
+    ReadObjCDefinitionData(ID->data(), Record, Idx);
+    ObjCInterfaceDecl *Canon = ID->getCanonicalDecl();
+    if (Canon->Data.getPointer()) {
+      // If we already have a definition, keep the definition invariant and
+      // merge the data.
+      MergeDefinitionData(Canon, std::move(ID->data()));
+      ID->Data = Canon->Data;
+    } else {
+      // Set the definition data of the canonical declaration, so other
+      // redeclarations will see it.
+      ID->getCanonicalDecl()->Data = ID->Data;
     
-    // Read the directly referenced protocols and their SourceLocations.
-    unsigned NumProtocols = Record[Idx++];
-    SmallVector<ObjCProtocolDecl *, 16> Protocols;
-    Protocols.reserve(NumProtocols);
-    for (unsigned I = 0; I != NumProtocols; ++I)
-      Protocols.push_back(ReadDeclAs<ObjCProtocolDecl>(Record, Idx));
-    SmallVector<SourceLocation, 16> ProtoLocs;
-    ProtoLocs.reserve(NumProtocols);
-    for (unsigned I = 0; I != NumProtocols; ++I)
-      ProtoLocs.push_back(ReadSourceLocation(Record, Idx));
-    ID->setProtocolList(Protocols.data(), NumProtocols, ProtoLocs.data(),
-                        Reader.getContext());
-  
-    // Read the transitive closure of protocols referenced by this class.
-    NumProtocols = Record[Idx++];
-    Protocols.clear();
-    Protocols.reserve(NumProtocols);
-    for (unsigned I = 0; I != NumProtocols; ++I)
-      Protocols.push_back(ReadDeclAs<ObjCProtocolDecl>(Record, Idx));
-    ID->data().AllReferencedProtocols.set(Protocols.data(), NumProtocols,
-                                          Reader.getContext());
-  
-    // We will rebuild this list lazily.
-    ID->setIvarList(nullptr);
+      // We will rebuild this list lazily.
+      ID->setIvarList(nullptr);
+    }
 
     // Note that we have deserialized a definition.
     Reader.PendingDefinitions.insert(ID);
@@ -3502,7 +3522,10 @@ Decl *ASTReader::ReadDeclRecord(DeclID ID) {
 
   // Load the categories after recursive loading is finished.
   if (ObjCInterfaceDecl *Class = dyn_cast<ObjCInterfaceDecl>(D))
-    if (Class->isThisDeclarationADefinition())
+    // If we already have a definition when deserializing the ObjCInterfaceDecl,
+    // we put the Decl in PendingDefinitions so we can pull the categories here.
+    if (Class->isThisDeclarationADefinition() ||
+        PendingDefinitions.count(Class))
       loadObjCCategories(ID, Class);
   
   // If we have deserialized a declaration that has a definition the
diff --git a/clang/test/Modules/Inputs/lookup-assert/Base.h b/clang/test/Modules/Inputs/lookup-assert/Base.h
new file mode 100644
index 00000000000..67e66183ca1
--- /dev/null
+++ b/clang/test/Modules/Inputs/lookup-assert/Base.h
@@ -0,0 +1,3 @@
+@interface BaseInterface
+- (void) test;
+@end
diff --git a/clang/test/Modules/Inputs/lookup-assert/Derive.h b/clang/test/Modules/Inputs/lookup-assert/Derive.h
new file mode 100644
index 00000000000..313a96188d2
--- /dev/null
+++ b/clang/test/Modules/Inputs/lookup-assert/Derive.h
@@ -0,0 +1,3 @@
+#include "Base.h"
+@interface DerivedInterface : BaseInterface
+@end
diff --git a/clang/test/Modules/Inputs/lookup-assert/H3.h b/clang/test/Modules/Inputs/lookup-assert/H3.h
new file mode 100644
index 00000000000..3d8f878905d
--- /dev/null
+++ b/clang/test/Modules/Inputs/lookup-assert/H3.h
@@ -0,0 +1 @@
+#include "Base.h"
diff --git a/clang/test/Modules/Inputs/lookup-assert/module.map b/clang/test/Modules/Inputs/lookup-assert/module.map
new file mode 100644
index 00000000000..e8a89eb095a
--- /dev/null
+++ b/clang/test/Modules/Inputs/lookup-assert/module.map
@@ -0,0 +1,4 @@
+module X {
+  header "H3.h"
+  export *
+}
diff --git a/clang/test/Modules/Inputs/objc-category-2/Base.h b/clang/test/Modules/Inputs/objc-category-2/Base.h
new file mode 100644
index 00000000000..9bd8b17a87e
--- /dev/null
+++ b/clang/test/Modules/Inputs/objc-category-2/Base.h
@@ -0,0 +1,3 @@
+@interface DVTSourceModel // expected-error {{duplicate interface definition for class}} \
+                          // expected-note {{previous definition is here}}
+@end
diff --git a/clang/test/Modules/Inputs/objc-category-2/Category.h b/clang/test/Modules/Inputs/objc-category-2/Category.h
new file mode 100644
index 00000000000..7cde9fb64cc
--- /dev/null
+++ b/clang/test/Modules/Inputs/objc-category-2/Category.h
@@ -0,0 +1,4 @@
+#include "Base.h"
+@interface DVTSourceModel(Additions)
+- (int)test:(int)item;
+@end
diff --git a/clang/test/Modules/Inputs/objc-category-2/H3.h b/clang/test/Modules/Inputs/objc-category-2/H3.h
new file mode 100644
index 00000000000..3d8f878905d
--- /dev/null
+++ b/clang/test/Modules/Inputs/objc-category-2/H3.h
@@ -0,0 +1 @@
+#include "Base.h"
diff --git a/clang/test/Modules/Inputs/objc-category-2/module.map b/clang/test/Modules/Inputs/objc-category-2/module.map
new file mode 100644
index 00000000000..833b189a338
--- /dev/null
+++ b/clang/test/Modules/Inputs/objc-category-2/module.map
@@ -0,0 +1,4 @@
+module X {
+  header "Category.h"
+  export *
+}
diff --git a/clang/test/Modules/Inputs/objc-category-3/Base.h b/clang/test/Modules/Inputs/objc-category-3/Base.h
new file mode 100644
index 00000000000..44094643b34
--- /dev/null
+++ b/clang/test/Modules/Inputs/objc-category-3/Base.h
@@ -0,0 +1,2 @@
+@interface DVTSourceModel
+@end
diff --git a/clang/test/Modules/Inputs/objc-category-3/Category.h b/clang/test/Modules/Inputs/objc-category-3/Category.h
new file mode 100644
index 00000000000..7cde9fb64cc
--- /dev/null
+++ b/clang/test/Modules/Inputs/objc-category-3/Category.h
@@ -0,0 +1,4 @@
+#include "Base.h"
+@interface DVTSourceModel(Additions)
+- (int)test:(int)item;
+@end
diff --git a/clang/test/Modules/Inputs/objc-category-3/Category_B.h b/clang/test/Modules/Inputs/objc-category-3/Category_B.h
new file mode 100644
index 00000000000..d67f94b3a9c
--- /dev/null
+++ b/clang/test/Modules/Inputs/objc-category-3/Category_B.h
@@ -0,0 +1,4 @@
+#include "Base.h"
+@interface DVTSourceModel(AdditionsB)
+- (int)testB:(int)item matchingMask:(int)mask;
+@end
diff --git a/clang/test/Modules/Inputs/objc-category-3/H3.h b/clang/test/Modules/Inputs/objc-category-3/H3.h
new file mode 100644
index 00000000000..3d8f878905d
--- /dev/null
+++ b/clang/test/Modules/Inputs/objc-category-3/H3.h
@@ -0,0 +1 @@
+#include "Base.h"
diff --git a/clang/test/Modules/Inputs/objc-category-3/module.map b/clang/test/Modules/Inputs/objc-category-3/module.map
new file mode 100644
index 00000000000..c53d5dc6efa
--- /dev/null
+++ b/clang/test/Modules/Inputs/objc-category-3/module.map
@@ -0,0 +1,4 @@
+module X {
+  header "Category_B.h"
+  export *
+}
diff --git a/clang/test/Modules/Inputs/objc-category/Base.h b/clang/test/Modules/Inputs/objc-category/Base.h
new file mode 100644
index 00000000000..44094643b34
--- /dev/null
+++ b/clang/test/Modules/Inputs/objc-category/Base.h
@@ -0,0 +1,2 @@
+@interface DVTSourceModel
+@end
diff --git a/clang/test/Modules/Inputs/objc-category/Category.h b/clang/test/Modules/Inputs/objc-category/Category.h
new file mode 100644
index 00000000000..7cde9fb64cc
--- /dev/null
+++ b/clang/test/Modules/Inputs/objc-category/Category.h
@@ -0,0 +1,4 @@
+#include "Base.h"
+@interface DVTSourceModel(Additions)
+- (int)test:(int)item;
+@end
diff --git a/clang/test/Modules/Inputs/objc-category/H3.h b/clang/test/Modules/Inputs/objc-category/H3.h
new file mode 100644
index 00000000000..3d8f878905d
--- /dev/null
+++ b/clang/test/Modules/Inputs/objc-category/H3.h
@@ -0,0 +1 @@
+#include "Base.h"
diff --git a/clang/test/Modules/Inputs/objc-category/module.map b/clang/test/Modules/Inputs/objc-category/module.map
new file mode 100644
index 00000000000..e8a89eb095a
--- /dev/null
+++ b/clang/test/Modules/Inputs/objc-category/module.map
@@ -0,0 +1,4 @@
+module X {
+  header "H3.h"
+  export *
+}
diff --git a/clang/test/Modules/lookup-assert.m b/clang/test/Modules/lookup-assert.m
new file mode 100644
index 00000000000..2697fb15d01
--- /dev/null
+++ b/clang/test/Modules/lookup-assert.m
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs/lookup-assert %s -verify
+// expected-no-diagnostics
+
+#include "Derive.h"
+#import <H3.h>
+@implementation DerivedInterface
+- (void)test {
+}
+@end
diff --git a/clang/test/Modules/objc-category-2.m b/clang/test/Modules/objc-category-2.m
new file mode 100644
index 00000000000..3a6f52d6105
--- /dev/null
+++ b/clang/test/Modules/objc-category-2.m
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs/objc-category-2 %s -verify -fobjc-arc
+
+// We have a definition of category and the base interface imported from a
+// module, definition for the base interface is also textually included.
+// Currently we emit an error "duplicate interface definition".
+#import <Category.h>
+#include "H3.h"
+
+void test(DVTSourceModel *m) {
+  [m test:1];
+}
diff --git a/clang/test/Modules/objc-category-3.m b/clang/test/Modules/objc-category-3.m
new file mode 100644
index 00000000000..b5162bffbfe
--- /dev/null
+++ b/clang/test/Modules/objc-category-3.m
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs/objc-category-3 %s -verify -fobjc-arc
+// expected-no-diagnostics
+
+// We have a definition of the base interface textually included from
+// Category.h, the definition is also in the module that includes the base
+// interface. We should be able to see both categories in the TU.
+#include "Category.h" 
+#import <Category_B.h>
+
+void test(DVTSourceModel *m) {
+  [m test:1];
+  [m testB:1 matchingMask:2];
+}
diff --git a/clang/test/Modules/objc-category.m b/clang/test/Modules/objc-category.m
new file mode 100644
index 00000000000..944c7ea625d
--- /dev/null
+++ b/clang/test/Modules/objc-category.m
@@ -0,0 +1,13 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs/objc-category %s -verify -fobjc-arc
+// expected-no-diagnostics
+
+// We have a definition of the base interface textually included from
+// Category.h, the definition is also in the module that includes the base
+// interface. We should be able to see the category in the TU.
+#include "Category.h" 
+#import <H3.h>
+
+void test(DVTSourceModel *m) {
+  [m test:1];
+}