diff --git a/clang/include/clang/AST/DeclObjC.h b/clang/include/clang/AST/DeclObjC.h --- a/clang/include/clang/AST/DeclObjC.h +++ b/clang/include/clang/AST/DeclObjC.h @@ -1150,6 +1150,7 @@ class ObjCInterfaceDecl : public ObjCContainerDecl , public Redeclarable { friend class ASTContext; + friend class ASTReader; /// TypeForDecl - This indicates the Type object that represents this /// TypeDecl. It is a cache maintained by ASTContext::getObjCInterfaceType diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1129,6 +1129,11 @@ llvm::SmallDenseMap, 2> PendingEnumOdrMergeFailures; + /// ObjCInterfaceDecl in which we found an ODR violation. + llvm::SmallDenseMap, 2> + PendingObjCInterfaceOdrMergeFailures; + /// DeclContexts in which we have diagnosed an ODR violation. llvm::SmallPtrSet DiagnosedOdrMergeFailures; diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -42,6 +42,7 @@ #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" @@ -9400,7 +9401,8 @@ 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 @@ -9422,6 +9424,17 @@ } } + // 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(); @@ -9529,7 +9542,7 @@ } if (OdrMergeFailures.empty() && FunctionOdrMergeFailures.empty() && - EnumOdrMergeFailures.empty()) + EnumOdrMergeFailures.empty() && ObjCInterfaceOdrMergeFailures.empty()) return; // Ensure we don't accidentally recursively enter deserialization while @@ -11117,6 +11130,14 @@ } } + 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 { diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -1152,6 +1152,8 @@ 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(); @@ -1183,9 +1185,12 @@ 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) { diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -761,6 +761,8 @@ 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()) diff --git a/clang/test/Modules/compare-objc-interface.m b/clang/test/Modules/compare-objc-interface.m --- a/clang/test/Modules/compare-objc-interface.m +++ b/clang/test/Modules/compare-objc-interface.m @@ -11,13 +11,21 @@ // 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 @@ -54,14 +62,23 @@ 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" +# include "first-empty.h" +# ifdef TEST_MODULAR +# include "second-modular.h" +# else +# include "second-textual.h" +# endif #endif #if defined(FIRST) @@ -81,6 +98,6 @@ @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 diff --git a/clang/test/Modules/interface-diagnose-missing-import.m b/clang/test/Modules/interface-diagnose-missing-import.m --- a/clang/test/Modules/interface-diagnose-missing-import.m +++ b/clang/test/Modules/interface-diagnose-missing-import.m @@ -1,7 +1,9 @@ // 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; diff --git a/clang/test/Modules/method_pool.m b/clang/test/Modules/method_pool.m --- a/clang/test/Modules/method_pool.m +++ b/clang/test/Modules/method_pool.m @@ -28,6 +28,9 @@ } @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];