[modules] Proof of concept in using ODR hash to detect mismatching duplicate ObjCInterfaceDecl during deserialization.

[vsapsai]

Superseded by D140073.

clang/include/clang/AST/DeclObjC.h

 ///   Unlike C/C++, forward class declarations are accomplished with \@class.
 ///   Unlike C/C++, \@class allows for a list of classes to be forward declared.
 ///   Unlike C++, ObjC is a single-rooted class model. In Cocoa, classes
 ///   typically inherit from NSObject (an exception is NSProxy).
 ///
 class ObjCInterfaceDecl : public ObjCContainerDecl
                         , public Redeclarable<ObjCInterfaceDecl> {
   friend class ASTContext;
+  friend class ASTReader;
 
   /// TypeForDecl - This indicates the Type object that represents this
   /// TypeDecl.  It is a cache maintained by ASTContext::getObjCInterfaceType
   mutable const Type *TypeForDecl = nullptr;
 
   struct DefinitionData {
     /// The definition of this class, for quick access from any
     /// declaration.

clang/include/clang/Serialization/ASTReader.h

private:
   /// Function definitions in which we found an ODR violation.
   llvm::SmallDenseMap<FunctionDecl *, llvm::SmallVector<FunctionDecl *, 2>, 2>
       PendingFunctionOdrMergeFailures;
 
   /// Enum definitions in which we found an ODR violation.
   llvm::SmallDenseMap<EnumDecl *, llvm::SmallVector<EnumDecl *, 2>, 2>
       PendingEnumOdrMergeFailures;
 
+  /// ObjCInterfaceDecl in which we found an ODR violation.
+  llvm::SmallDenseMap<ObjCInterfaceDecl *,
+                      llvm::SmallVector<ObjCInterfaceDecl *, 2>, 2>
+      PendingObjCInterfaceOdrMergeFailures;
+
   /// DeclContexts in which we have diagnosed an ODR violation.
   llvm::SmallPtrSet<DeclContext*, 2> DiagnosedOdrMergeFailures;
 
   /// The set of Objective-C categories that have been deserialized
   /// since the last time the declaration chains were linked.
   llvm::SmallPtrSet<ObjCCategoryDecl *, 16> CategoriesDeserialized;
 
   /// The set of Objective-C class definitions that have already been

clang/lib/Serialization/ASTReader.cpp

 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeLocVisitor.h"
 #include "clang/AST/UnresolvedSet.h"
 #include "clang/Basic/CommentOptions.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/DiagnosticSema.h"
 #include "clang/Basic/ExceptionSpecificationType.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/ObjCRuntime.h"
void ASTReader::finishPendingActions() {
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
     getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
 }
 
 void ASTReader::diagnoseOdrViolations() {
   if (PendingOdrMergeFailures.empty() && PendingOdrMergeChecks.empty() &&
       PendingFunctionOdrMergeFailures.empty() &&
-      PendingEnumOdrMergeFailures.empty())
+      PendingEnumOdrMergeFailures.empty() &&
+      PendingObjCInterfaceOdrMergeFailures.empty())
     return;
 
   // Trigger the import of the full definition of each class that had any
   // odr-merging problems, so we can produce better diagnostics for them.
   // These updates may in turn find and diagnose some ODR failures, so take
   // ownership of the set first.
   auto OdrMergeFailures = std::move(PendingOdrMergeFailures);
   PendingOdrMergeFailures.clear();
   for (auto &Merge : OdrMergeFailures) {
     Merge.first->buildLookup();
     Merge.first->decls_begin();
     Merge.first->bases_begin();
     Merge.first->vbases_begin();
     for (auto &RecordPair : Merge.second) {
       auto *RD = RecordPair.first;
       RD->decls_begin();
       RD->bases_begin();
       RD->vbases_begin();
     }
   }
 
+  // Trigger the import of the full interface definition.
+  auto ObjCInterfaceOdrMergeFailures =
+      std::move(PendingObjCInterfaceOdrMergeFailures);
+  PendingObjCInterfaceOdrMergeFailures.clear();
+  for (auto &Merge : ObjCInterfaceOdrMergeFailures) {
+    Merge.first->decls_begin();
+    for (auto &Interface : Merge.second) {
+      Interface->decls_begin();
+    }
+  }
+
   // Trigger the import of functions.
   auto FunctionOdrMergeFailures = std::move(PendingFunctionOdrMergeFailures);
   PendingFunctionOdrMergeFailures.clear();
   for (auto &Merge : FunctionOdrMergeFailures) {
     Merge.first->buildLookup();
     Merge.first->decls_begin();
     Merge.first->getBody();
     for (auto &FD : Merge.second) {
if (!Found) {
             << Candidates[I];
       }
 
       DiagnosedOdrMergeFailures.insert(CanonDef);
     }
   }
 
   if (OdrMergeFailures.empty() && FunctionOdrMergeFailures.empty() &&
-      EnumOdrMergeFailures.empty())
+      EnumOdrMergeFailures.empty() && ObjCInterfaceOdrMergeFailures.empty())
     return;
 
   // Ensure we don't accidentally recursively enter deserialization while
   // we're producing our diagnostics.
   Deserializing RecursionGuard(this);
 
   // Common code for hashing helpers.
   ODRHash Hash;
if (!Diagnosed) {
       // FIXME: Indicate which modules had instantiations of this definition.
       // FIXME: How can this even happen?
       Diag(Merge.first->getLocation(),
            diag::err_module_odr_violation_different_instantiations)
         << Merge.first;
     }
   }
 
+  for (auto &Merge : ObjCInterfaceOdrMergeFailures) {
+    for (auto &Interface : Merge.second) {
+      Diag(Interface->getLocation(), diag::err_duplicate_class_def)
+          << Interface->getDeclName();
+      Diag(Merge.first->getLocation(), diag::note_previous_definition);
+    }
+  }
+
   // Issue ODR failures diagnostics for functions.
   for (auto &Merge : FunctionOdrMergeFailures) {
     enum ODRFunctionDifference {
       ReturnType,
       ParameterName,
       ParameterType,
       ParameterSingleDefaultArgument,
       ParameterDifferentDefaultArgument,

clang/lib/Serialization/ASTReaderDecl.cpp

 
 void ASTDeclReader::ReadObjCDefinitionData(
          struct ObjCInterfaceDecl::DefinitionData &Data) {
   // Read the superclass.
   Data.SuperClassTInfo = readTypeSourceInfo();
 
   Data.EndLoc = readSourceLocation();
   Data.HasDesignatedInitializers = Record.readInt();
+  Data.ODRHash = Record.readInt();
+  Data.HasODRHash = true;
 
   // Read the directly referenced protocols and their SourceLocations.
   unsigned NumProtocols = Record.readInt();
   SmallVector<ObjCProtocolDecl *, 16> Protocols;
   Protocols.reserve(NumProtocols);
   for (unsigned I = 0; I != NumProtocols; ++I)
     Protocols.push_back(readDeclAs<ObjCProtocolDecl>());
   SmallVector<SourceLocation, 16> ProtoLocs;

 
 void ASTDeclReader::MergeDefinitionData(ObjCInterfaceDecl *D,
          struct ObjCInterfaceDecl::DefinitionData &&NewDD) {
   struct ObjCInterfaceDecl::DefinitionData &DD = D->data();
   if (DD.Definition != NewDD.Definition) {
     Reader.MergedDeclContexts.insert(
         std::make_pair(NewDD.Definition, DD.Definition));
     Reader.mergeDefinitionVisibility(DD.Definition, NewDD.Definition);
+    if (D->getODRHash() != NewDD.Definition->getODRHash())
+      Reader.PendingObjCInterfaceOdrMergeFailures[D].push_back(
+          NewDD.Definition);
   }
 
-  // FIXME: odr checking?
+  // FIXME: odr checking for categories and extensions?
 }
 
 void ASTDeclReader::VisitObjCInterfaceDecl(ObjCInterfaceDecl *ID) {
   RedeclarableResult Redecl = VisitRedeclarable(ID);
   VisitObjCContainerDecl(ID);
   DeferredTypeID = Record.getGlobalTypeID(Record.readInt());
   mergeRedeclarable(ID, Redecl);
 

clang/lib/Serialization/ASTWriterDecl.cpp

void ASTDeclWriter::VisitObjCInterfaceDecl(ObjCInterfaceDecl *D) {
   if (D->isThisDeclarationADefinition()) {
     // Write the DefinitionData
     ObjCInterfaceDecl::DefinitionData &Data = D->data();
 
     Record.AddTypeSourceInfo(D->getSuperClassTInfo());
     Record.AddSourceLocation(D->getEndOfDefinitionLoc());
     Record.push_back(Data.HasDesignatedInitializers);
 
+    Record.push_back(D->getODRHash());
+
     // Write out the protocols that are directly referenced by the @interface.
     Record.push_back(Data.ReferencedProtocols.size());
     for (const auto *P : D->protocols())
       Record.AddDeclRef(P);
     for (const auto &PL : D->protocol_locs())
       Record.AddSourceLocation(PL);
 
     // Write out the protocols that are transitively referenced.

clang/test/Modules/compare-objc-interface.m

 // RUN: rm -rf %t
 // RUN: split-file %s %t
 
 // Build first header file
 // RUN: echo "#define FIRST" >> %t/include/first.h
 // RUN: cat %t/test.m        >> %t/include/first.h
 // RUN: echo "#undef FIRST"  >> %t/include/first.h
 
 // Build second header file
 // RUN: echo "#define SECOND" >> %t/include/second-textual.h
 // RUN: cat %t/test.m         >> %t/include/second-textual.h
 // RUN: echo "#undef SECOND"  >> %t/include/second-textual.h
 
+// Build second modular header file
+// RUN: echo "#define SECOND" >> %t/include/second-modular.h
+// RUN: cat %t/test.m         >> %t/include/second-modular.h
+// RUN: echo "#undef SECOND"  >> %t/include/second-modular.h
+
 // Test that each header can compile
 // RUN: %clang_cc1 -fsyntax-only -x objective-c %t/include/first.h -fblocks -fobjc-arc
 // RUN: %clang_cc1 -fsyntax-only -x objective-c %t/include/second-textual.h -fblocks -fobjc-arc
+// RUN: %clang_cc1 -fsyntax-only -x objective-c %t/include/second-modular.h -fblocks -fobjc-arc
 
 // Run test
 // RUN: %clang_cc1 -I%t/include -verify %t/test.m -fblocks -fobjc-arc \
 // RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+// RUN: %clang_cc1 -I%t/include -verify %t/test.m -fblocks -fobjc-arc -DTEST_MODULAR=1 \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
 
 // In non-modular case we don't allow interface redefinitions. But with modules
 // previous definition can come from a hidden [sub]module. And in this case we
 // allow a new definition if it is equivalent to the hidden one.
 //
 // This test case is to verify equivalence checks.
 
 //--- include/common.h
module First {
   module Empty {
     header "first-empty.h"
   }
   module InitiallyHidden {
     header "first.h"
     export *
   }
 }
+module Second {
+  header "second-modular.h"
+  export *
+}
 
 //--- test.m
 #if defined(FIRST) || defined(SECOND)
-#include "common.h"
+# include "common.h"
 #endif
+
 #if !defined(FIRST) && !defined(SECOND)
-#include "first-empty.h"
-#include "second-textual.h"
-#endif
+# include "first-empty.h"
+# ifdef TEST_MODULAR
+#  include "second-modular.h"
+# else
+#  include "second-textual.h"
+# endif
+#endif
 
 #if defined(FIRST)
 @class CompareForwardDeclaration1;
 @interface CompareForwardDeclaration2: NSObject @end
 #elif defined(SECOND)
 @interface CompareForwardDeclaration1: NSObject @end
 @class CompareForwardDeclaration2;
 #else
 CompareForwardDeclaration1 *compareForwardDeclaration1;
 CompareForwardDeclaration2 *compareForwardDeclaration2;
 #endif
 
 #if defined(FIRST)
 @interface CompareSuperClass : NSObject @end
 #elif defined(SECOND)
 @interface CompareSuperClass : CommonClass @end
 #else
 CompareSuperClass *compareSuperClass;
-// expected-error@second-textual.h:* {{duplicate interface definition for class 'CompareSuperClass'}}
+// expected-error@*:* {{duplicate interface definition for class 'CompareSuperClass'}}
 // expected-note@first.h:* {{previous definition is here}}
 #endif

clang/test/Modules/interface-diagnose-missing-import.m

 // RUN: rm -rf %t
 // RUN: %clang_cc1 %s -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F%S/Inputs/interface-diagnose-missing-import -verify
 // expected-no-diagnostics
-@interface Buggy
+@interface NSObject
+@end
+@interface Buggy: NSObject
 @end
 
 @import Foo.Bar;
 
 // No diagnostic for inaccessible 'Buggy' definition because we have another definition right in this file.
 @interface Buggy (MyExt)
 @end

clang/test/Modules/method_pool.m

void testMethod4(id object) {
   [object method4]; // expected-warning{{instance method '-method4' not found (return type defaults to 'id')}}
 } 
 
 void testMethod5(id object, D* d) {
   [object method5:d];
 }
 
 @import MethodPoolB;
+//FIXME: main definition should come from MethodPoolA and definition in MethodPoolB should be a duplicate
+// expected-error@MethodPoolA.h:* {{duplicate interface definition for class 'B'}}
+// expected-note@MethodPoolB.h:* {{previous definition is here}}
 
 void testMethod1Again(id object) {
   [object method1];
 }
 
 void testMethod2Again(id object) {
   [object method2:1]; // expected-warning{{multiple methods named 'method2:' found}}
 }