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 @@ -1539,6 +1539,13 @@ /// a forward declaration (\@class) to a definition (\@interface). void startDefinition(); + /// Starts the definition without sharing it with other redeclarations. + /// Such definition shouldn't be used for anything but only to compare if + /// a duplicate is compatible with previous definition or if it is + /// a distinct duplicate. + void startDuplicateDefinitionForComparison(); + void mergeDuplicateDefinitionWithCommon(const ObjCInterfaceDecl *Definition); + /// Retrieve the superclass type. const ObjCObjectType *getSuperClassType() const { if (TypeSourceInfo *TInfo = getSuperClassTInfo()) 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 @@ -9750,7 +9750,7 @@ ArrayRef SuperTypeArgs, SourceRange SuperTypeArgsRange, Decl *const *ProtoRefs, unsigned NumProtoRefs, const SourceLocation *ProtoLocs, SourceLocation EndProtoLoc, - const ParsedAttributesView &AttrList); + const ParsedAttributesView &AttrList, SkipBodyInfo *SkipBody); void ActOnSuperClassOfClassInterface(Scope *S, SourceLocation AtInterfaceLoc, 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 @@ -627,6 +627,17 @@ } } +void ObjCInterfaceDecl::startDuplicateDefinitionForComparison() { + Data.setPointer(nullptr); + allocateDefinitionData(); + // Don't propagate data to other redeclarations. +} + +void ObjCInterfaceDecl::mergeDuplicateDefinitionWithCommon( + const ObjCInterfaceDecl *Definition) { + Data = Definition->Data; +} + ObjCIvarDecl *ObjCInterfaceDecl::lookupInstanceVariable(IdentifierInfo *ID, ObjCInterfaceDecl *&clsDeclared) { // FIXME: Should make sure no callers ever do this. 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 @@ -13,6 +13,7 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/PrettyDeclStackTrace.h" #include "clang/Basic/CharInfo.h" +#include "clang/Basic/DiagnosticSema.h" #include "clang/Basic/TargetInfo.h" #include "clang/Parse/ParseDiagnostic.h" #include "clang/Parse/Parser.h" @@ -353,17 +354,29 @@ Actions.ActOnTypedefedProtocols(protocols, protocolLocs, superClassId, superClassLoc); + Sema::SkipBodyInfo SkipBody; ObjCInterfaceDecl *ClsType = Actions.ActOnStartClassInterface( getCurScope(), AtLoc, nameId, nameLoc, typeParameterList, superClassId, superClassLoc, typeArgs, SourceRange(typeArgsLAngleLoc, typeArgsRAngleLoc), protocols.data(), - protocols.size(), protocolLocs.data(), EndProtoLoc, attrs); + protocols.size(), protocolLocs.data(), EndProtoLoc, attrs, &SkipBody); if (Tok.is(tok::l_brace)) ParseObjCClassInstanceVariables(ClsType, tok::objc_protected, AtLoc); ParseObjCInterfaceDeclList(tok::objc_interface, ClsType); + if (SkipBody.CheckSameAsPrevious) { + if (Actions.ActOnDuplicateDefinition(SkipBody.Previous, SkipBody)) { + ClsType->mergeDuplicateDefinitionWithCommon( + cast(SkipBody.Previous)->getDefinition()); + } else { + Diag(AtLoc, diag::err_duplicate_class_def) << ClsType->getDeclName(); + Diag(SkipBody.Previous->getLocation(), diag::note_previous_definition); + ClsType->setInvalidDecl(); + } + } + return ClsType; } diff --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp --- a/clang/lib/Sema/SemaDeclObjC.cpp +++ b/clang/lib/Sema/SemaDeclObjC.cpp @@ -978,7 +978,7 @@ ArrayRef SuperTypeArgs, SourceRange SuperTypeArgsRange, Decl *const *ProtoRefs, unsigned NumProtoRefs, const SourceLocation *ProtoLocs, SourceLocation EndProtoLoc, - const ParsedAttributesView &AttrList) { + const ParsedAttributesView &AttrList, SkipBodyInfo *SkipBody) { assert(ClassName && "Missing class identifier"); // Check for another declaration kind with the same name. @@ -1057,10 +1057,16 @@ if (PrevIDecl) { // Class already seen. Was it a definition? if (ObjCInterfaceDecl *Def = PrevIDecl->getDefinition()) { - Diag(AtInterfaceLoc, diag::err_duplicate_class_def) - << PrevIDecl->getDeclName(); - Diag(Def->getLocation(), diag::note_previous_definition); - IDecl->setInvalidDecl(); + if (SkipBody && !hasVisibleDefinition(Def)) { + SkipBody->CheckSameAsPrevious = true; + SkipBody->New = IDecl; + SkipBody->Previous = Def; + } else { + Diag(AtInterfaceLoc, diag::err_duplicate_class_def) + << PrevIDecl->getDeclName(); + Diag(Def->getLocation(), diag::note_previous_definition); + IDecl->setInvalidDecl(); + } } } @@ -1075,7 +1081,9 @@ // Start the definition of this class. If we're in a redefinition case, there // may already be a definition, so we'll end up adding to it. - if (!IDecl->hasDefinition()) + if (SkipBody && SkipBody->CheckSameAsPrevious) + IDecl->startDuplicateDefinitionForComparison(); + else if (!IDecl->hasDefinition()) IDecl->startDefinition(); if (SuperName) { diff --git a/clang/test/Modules/hidden-duplicates.m b/clang/test/Modules/hidden-duplicates.m new file mode 100644 --- /dev/null +++ b/clang/test/Modules/hidden-duplicates.m @@ -0,0 +1,62 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -I %t/include %t/test.m -verify \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache +// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -I %t/include %t/test.m -verify -DTEST_MAKE_HIDDEN_VISIBLE=1 \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache +// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -I %t/include %t/test.m -verify -x objective-c++ \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache +// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -I %t/include %t/test.m -verify -DTEST_MAKE_HIDDEN_VISIBLE=1 -x objective-c++ \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache + +// Test parsing duplicate Objective-C entities when a previous entity is defined +// in a hidden [sub]module and cannot be used. +// +// Testing with header guards and includes on purpose as tracking imports in +// modules is a separate issue. + +//--- include/textual.h +#ifndef TEXTUAL_H +#define TEXTUAL_H + +__attribute__((objc_root_class)) +@interface NSObject { +@public + int ivarName; +} +@property (assign, nonatomic) int propName; +@end + +@class ForwardDeclaredClassWithoutDefinition; + +#endif + +//--- include/empty.h +//--- include/initially_hidden.h +#include "textual.h" + +//--- include/module.modulemap +module Piecewise { + module Empty { + header "empty.h" + } + module InitiallyHidden { + header "initially_hidden.h" + export * + } +} + +//--- test.m +// Including empty.h loads the entire module Piecewise but keeps InitiallyHidden hidden. +#include "empty.h" +#include "textual.h" +#ifdef TEST_MAKE_HIDDEN_VISIBLE +#include "initially_hidden.h" +#endif +// expected-no-diagnostics + +void testAccessingInterface(NSObject *obj) { + obj->ivarName = obj.propName; +} + +void testForwardDecl(ForwardDeclaredClassWithoutDefinition *obj);