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 @@ -1207,6 +1207,12 @@ /// One of the \c InheritedDesignatedInitializersState enumeratos. mutable unsigned InheritedDesignatedInitializers : 2; + /// Tracks whether a ODR hash has been computed for this interface. + unsigned HasODRHash : 1; + + /// A hash of parts of the class to help in ODR checking. + unsigned ODRHash = 0; + /// The location of the last location in this declaration, before /// the properties/methods. For example, this will be the '>', '}', or /// identifier, @@ -1215,7 +1221,7 @@ DefinitionData() : ExternallyCompleted(false), IvarListMissingImplementation(true), HasDesignatedInitializers(false), - InheritedDesignatedInitializers(IDI_Unknown) {} + InheritedDesignatedInitializers(IDI_Unknown), HasODRHash(false) {} }; /// The type parameters associated with this class, if any. @@ -1904,12 +1910,19 @@ const Type *getTypeForDecl() const { return TypeForDecl; } void setTypeForDecl(const Type *TD) const { TypeForDecl = TD; } + /// Get precomputed ODRHash or add a new one. + unsigned getODRHash(); + static bool classof(const Decl *D) { return classofKind(D->getKind()); } static bool classofKind(Kind K) { return K == ObjCInterface; } private: const ObjCInterfaceDecl *findInterfaceWithDesignatedInitializers() const; bool inheritsDesignatedInitializers() const; + + /// True if a valid hash is stored in ODRHash. + bool hasODRHash() const; + void setHasODRHash(bool HasHash); }; /// ObjCIvarDecl - Represents an ObjC instance variable. In general, ObjC diff --git a/clang/include/clang/AST/ODRHash.h b/clang/include/clang/AST/ODRHash.h --- a/clang/include/clang/AST/ODRHash.h +++ b/clang/include/clang/AST/ODRHash.h @@ -55,6 +55,10 @@ // more information than the AddDecl class. void AddCXXRecordDecl(const CXXRecordDecl *Record); + // Use this for ODR checking ObjC interfaces. This + // method compares more information than the AddDecl class. + void AddObjCInterfaceDecl(const ObjCInterfaceDecl *Interface); + // Use this for ODR checking functions between modules. This method compares // more information than the AddDecl class. SkipBody will process the // hash as if the function has no body. diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -3292,6 +3292,20 @@ /// in case of a structural mismatch. bool ActOnDuplicateDefinition(Decl *Prev, SkipBodyInfo &SkipBody); + /// Check ODR hashes for C/ObjC when merging types from modules. + /// Differently from C++, actually parse the body and reject in case + /// of a mismatch. + template ::value>> + bool ActOnDuplicateDefinition(T *Duplicate, T *Previous) { + if (Duplicate->getODRHash() != Previous->getODRHash()) + return false; + + // Make the previous decl visible. + makeMergedDefinitionVisible(Previous); + return true; + } + typedef void *SkippedDefinitionContext; /// Invoked when we enter a tag definition that we're skipping. diff --git a/clang/lib/AST/DeclObjC.cpp b/clang/lib/AST/DeclObjC.cpp --- a/clang/lib/AST/DeclObjC.cpp +++ b/clang/lib/AST/DeclObjC.cpp @@ -16,6 +16,7 @@ #include "clang/AST/Attr.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" +#include "clang/AST/ODRHash.h" #include "clang/AST/Stmt.h" #include "clang/AST/Type.h" #include "clang/AST/TypeLoc.h" @@ -792,6 +793,33 @@ return Method; } +unsigned ObjCInterfaceDecl::getODRHash() { + assert(hasDefinition() && "ODRHash only for interfaces with definitions"); + + // Previously calculated hash is stored in DefinitionData. + if (hasODRHash()) + return data().ODRHash; + + // Only calculate hash on the first call of getODRHash per interface. + ODRHash Hasher; + Hasher.AddObjCInterfaceDecl(getDefinition()); + data().ODRHash = Hasher.CalculateHash(); + setHasODRHash(true); + + return data().ODRHash; +} + +bool ObjCInterfaceDecl::hasODRHash() const { + if (!hasDefinition()) + return false; + return data().HasODRHash; +} + +void ObjCInterfaceDecl::setHasODRHash(bool HasHash) { + assert(hasDefinition() && "Cannot set ODRHash without definition"); + data().HasODRHash = HasHash; +} + //===----------------------------------------------------------------------===// // ObjCMethodDecl //===----------------------------------------------------------------------===// diff --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp --- a/clang/lib/AST/ODRHash.cpp +++ b/clang/lib/AST/ODRHash.cpp @@ -517,6 +517,28 @@ } } +void ODRHash::AddObjCInterfaceDecl(const ObjCInterfaceDecl *IF) { + AddDecl(IF); + + ObjCInterfaceDecl *SuperClass = IF->getSuperClass(); + AddBoolean(SuperClass); + if (SuperClass) + ID.AddInteger(SuperClass->getODRHash()); + + //FIXME: Hash other interface-specific elements like protocols, etc. + + // Filter out sub-Decls which will not be processed in order to get an + // accurate count of Decl's. + llvm::SmallVector Decls; + for (Decl *SubDecl : IF->decls()) + if (isDeclToBeProcessed(SubDecl, IF)) + Decls.push_back(SubDecl); + + ID.AddInteger(Decls.size()); + for (auto *SubDecl : Decls) + AddSubDecl(SubDecl); +} + void ODRHash::AddFunctionDecl(const FunctionDecl *Function, bool SkipBody) { assert(Function && "Expecting non-null pointer."); diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp --- a/clang/lib/Parse/ParseObjc.cpp +++ b/clang/lib/Parse/ParseObjc.cpp @@ -367,9 +367,9 @@ ParseObjCInterfaceDeclList(tok::objc_interface, ClsType); if (SkipBody.CheckSameAsPrevious) { - if (Actions.ActOnDuplicateDefinition(SkipBody.Previous, SkipBody)) { - ClsType->mergeDuplicateDefinitionWithCommon( - cast(SkipBody.Previous)->getDefinition()); + auto *PreviousDef = cast(SkipBody.Previous); + if (Actions.ActOnDuplicateDefinition(ClsType, PreviousDef)) { + ClsType->mergeDuplicateDefinitionWithCommon(PreviousDef->getDefinition()); } else { Diag(AtLoc, diag::err_duplicate_class_def) << ClsType->getDeclName(); Diag(SkipBody.Previous->getLocation(), diag::note_previous_definition); diff --git a/clang/test/Modules/compare-objc-interface.m b/clang/test/Modules/compare-objc-interface.m new file mode 100644 --- /dev/null +++ b/clang/test/Modules/compare-objc-interface.m @@ -0,0 +1,86 @@ +// 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 + +// 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 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 + +// 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 +#ifndef COMMON_H +#define COMMON_H +__attribute__((objc_root_class)) +@interface NSObject +@end + +@interface CommonClass: NSObject +@end + +@protocol ProtoP @end +@protocol ProtoQ @end +#endif + +//--- include/first-empty.h +//--- include/module.modulemap +module Common { + header "common.h" + export * +} +module First { + module Empty { + header "first-empty.h" + } + module InitiallyHidden { + header "first.h" + export * + } +} + +//--- test.m +#if defined(FIRST) || defined(SECOND) +#include "common.h" +#endif +#if !defined(FIRST) && !defined(SECOND) +#include "first-empty.h" +#include "second-textual.h" +#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-note@first.h:* {{previous definition is here}} +#endif