Merge pull request #755 from bcardosolopes/apple-stable-20200108-odr-objc-protocol-no-referenced-list-odrhash

[ODRHash] Check references protocols as part of MergeDefintionData

Author: bcardosolopes

clang/lib/AST/ODRHash.cpp

@@ -499,11 +499,6 @@ void ODRHash::AddObjCInterfaceDecl(const ObjCInterfaceDecl *IF) {
   AddBoolean(SuperClass);
   if (SuperClass)
     ID.AddInteger(SuperClass->getODRHash());
-  ID.AddInteger(IF->getReferencedProtocols().size());
-  for (auto *P : IF->protocols()) {
-    unsigned ProtoHash = reinterpret_cast<ObjCProtocolDecl *>(P)->getODRHash();
-    ID.AddInteger(ProtoHash);
-  }
 
   // Filter out sub-Decls which will not be processed in order to get an
   // accurate count of Decl's.
@@ -524,13 +519,6 @@ void ODRHash::AddObjCProtocolDecl(const ObjCProtocolDecl *P) {
   for (auto *M : P->methods())
     reinterpret_cast<ObjCMethodDecl *>(M)->getODRHash();
 
-  // Store the hash of each referenced protocol.
-  ID.AddInteger(P->getReferencedProtocols().size());
-  for (auto *RefP : P->protocols()) {
-    unsigned RefHash = reinterpret_cast<ObjCProtocolDecl *>(RefP)->getODRHash();
-    ID.AddInteger(RefHash);
-  }
-
   // Filter out sub-Decls which will not be processed in order to get an
   // accurate count of Decl's.
   llvm::SmallVector<const Decl *, 16> Decls;

clang/lib/Serialization/ASTReaderDecl.cpp

@@ -1152,6 +1152,30 @@ void ASTDeclReader::ReadObjCDefinitionData(
                                   Reader.getContext());
 }
 
+static bool MergeCheckProtocolList(const ObjCProtocolList &FirstProtos,
+                                   const ObjCProtocolList &SecondProtos) {
+  if (FirstProtos.size() != SecondProtos.size())
+    return true;
+
+  for (unsigned I = 0, E = FirstProtos.size(); I != E; ++I) {
+    auto *FirstParam = FirstProtos[I];
+    auto *SecondParam = SecondProtos[I];
+    DeclarationName FirstParamName = FirstParam->getDeclName();
+    DeclarationName SecondParamName = SecondParam->getDeclName();
+
+    // If parameters match name but do not both have a definition, we
+    // can't disambiguate it during ODR diagnostic time, skip.
+    if (FirstParamName == SecondParamName &&
+        (FirstParam->hasDefinition() != SecondParam->hasDefinition()))
+      return false;
+
+    if (FirstParamName != SecondParamName)
+      return true;
+  }
+
+  return false;
+}
+
 void ASTDeclReader::MergeDefinitionData(ObjCInterfaceDecl *D,
          struct ObjCInterfaceDecl::DefinitionData &&NewDD) {
   bool DetectedOdrViolation = false;
@@ -1163,6 +1187,10 @@ void ASTDeclReader::MergeDefinitionData(ObjCInterfaceDecl *D,
       NewDD.CategoryList)
     return;
 
+  auto &FirstProtos = D->getReferencedProtocols();
+  auto &SecondProtos = NewDD.ReferencedProtocols;
+  if (MergeCheckProtocolList(FirstProtos, SecondProtos))
+    DetectedOdrViolation = true;
   if (D->getODRHash() != NewDD.ODRHash)
     DetectedOdrViolation = true;
 
@@ -1239,6 +1267,10 @@ void ASTDeclReader::MergeDefinitionData(ObjCProtocolDecl *D,
   bool DetectedOdrViolation = false;
   auto &DD = D->data();
 
+  auto &FirstProtos = D->getReferencedProtocols();
+  auto &SecondProtos = NewDD.ReferencedProtocols;
+  if (MergeCheckProtocolList(FirstProtos, SecondProtos))
+    DetectedOdrViolation = true;
   if (D->getODRHash() != NewDD.ODRHash)
     DetectedOdrViolation = true;
 

clang/test/Modules/odr_hash.m

@@ -46,6 +46,10 @@ @protocol P1
 @protocol P2
 @end
 
+@protocol UP1;
+@protocol UP2;
+@protocol UP3;
+
 @interface I1
 @end
 
@@ -623,6 +627,55 @@ @interface IMW5 <MW5> // No diagnostics: @required is the default.
 @end
 #endif
 
+#if defined(FIRST)
+@protocol PP1 <UP1>
+@end
+#elif defined(SECOND)
+@protocol UP1
+@end
+@protocol PP11 <UP1>
+@end
+#else
+@interface II0 <PP1>
+@end
+II0 *ii0;
+#endif
+
+#if defined(FIRST)
+@protocol PP2 <UP2>
+@end
+#elif defined(SECOND)
+@protocol PP2 <UP2>
+@end
+#else
+#endif
+
+#if defined(FIRST)
+@protocol PP3 <UP2>
+@end
+#elif defined(SECOND)
+@protocol PP3 <UP3>
+@end
+#else
+@protocol PP4 <PP3>
+@end
+// [email protected]:* {{'PP3' has different definitions in different modules; first difference is definition in module 'FirstModule' found with 1st protocol named 'UP2'}}
+// [email protected]:* {{but in 'SecondModule' found with 1st protocol named 'UP3'}}
+#endif
+
+#if defined(FIRST)
+@interface II1 <UP3>
+@end
+// [email protected]:* {{cannot find protocol definition for 'UP3'}}
+// [email protected]:* {{protocol 'UP3' has no definition}}
+#elif defined(SECOND)
+@protocol UP3
+@end
+@interface II1 <UP3>
+@end
+#else
+#endif
+
 // Keep macros contained to one file.
 #ifdef FIRST
 #undef FIRST