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 @@ -1951,7 +1951,10 @@ /// in; this is either the interface where the ivar was declared, or the /// interface the ivar is conceptually a part of in the case of synthesized /// ivars. - const ObjCInterfaceDecl *getContainingInterface() const; + ObjCInterfaceDecl *getContainingInterface(); + const ObjCInterfaceDecl *getContainingInterface() const { + return const_cast(this)->getContainingInterface(); + } ObjCIvarDecl *getNextIvar() { return NextIvar; } const ObjCIvarDecl *getNextIvar() const { return NextIvar; } @@ -2885,15 +2888,16 @@ } inline bool ObjCInterfaceDecl::isVisibleCategory(ObjCCategoryDecl *Cat) { - return Cat->isUnconditionallyVisible(); + return !Cat->isInvalidDecl() && Cat->isUnconditionallyVisible(); } inline bool ObjCInterfaceDecl::isVisibleExtension(ObjCCategoryDecl *Cat) { - return Cat->IsClassExtension() && Cat->isUnconditionallyVisible(); + return !Cat->isInvalidDecl() && Cat->IsClassExtension() && + Cat->isUnconditionallyVisible(); } inline bool ObjCInterfaceDecl::isKnownExtension(ObjCCategoryDecl *Cat) { - return Cat->IsClassExtension(); + return !Cat->isInvalidDecl() && Cat->IsClassExtension(); } } // namespace clang 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 @@ -1106,6 +1106,18 @@ /// been completed. std::deque PendingDeclContextInfos; + template + using DuplicateObjCDecls = std::pair; + + /// When resolving duplicate ivars from Objective-C extensions we don't error + /// out immediately but check if can merge identical extensions. Not checking + /// extensions for equality immediately because ivar deserialization isn't + /// over yet at that point. + llvm::SmallMapVector, + llvm::SmallVector, 4>, + 2> + PendingObjCExtensionIvarRedeclarations; + /// The set of NamedDecls that have been loaded, but are members of a /// context that has been merged into another context where the corresponding /// declaration is either missing or has not yet been loaded. 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 @@ -1647,6 +1647,11 @@ ObjCIvarDecl *curIvar = nullptr; if (!data().IvarList) { + // Force ivar deserialization upfront, before building IvarList. + (void)ivar_empty(); + for (const auto *Ext : known_extensions()) { + (void)Ext->ivar_empty(); + } if (!ivar_empty()) { ObjCInterfaceDecl::ivar_iterator I = ivar_begin(), E = ivar_end(); data().IvarList = *I; ++I; @@ -1838,8 +1843,8 @@ ObjCIvarDecl::None, nullptr, false); } -const ObjCInterfaceDecl *ObjCIvarDecl::getContainingInterface() const { - const auto *DC = cast(getDeclContext()); +ObjCInterfaceDecl *ObjCIvarDecl::getContainingInterface() { + auto *DC = cast(getDeclContext()); switch (DC->getKind()) { default: @@ -1849,7 +1854,7 @@ // Ivars can only appear in class extension categories. case ObjCCategory: { - const auto *CD = cast(DC); + auto *CD = cast(DC); assert(CD->IsClassExtension() && "invalid container for ivar!"); return CD->getClassInterface(); } 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 @@ -15,6 +15,7 @@ #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" #include "clang/AST/ASTMutationListener.h" +#include "clang/AST/ASTStructuralEquivalence.h" #include "clang/AST/ASTUnresolvedSet.h" #include "clang/AST/AbstractTypeReader.h" #include "clang/AST/Decl.h" @@ -42,6 +43,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" @@ -9193,7 +9195,8 @@ while (!PendingIdentifierInfos.empty() || !PendingFunctionTypes.empty() || !PendingIncompleteDeclChains.empty() || !PendingDeclChains.empty() || !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() || - !PendingUpdateRecords.empty()) { + !PendingUpdateRecords.empty() || + !PendingObjCExtensionIvarRedeclarations.empty()) { // If any identifiers with corresponding top-level declarations have // been loaded, load those declarations now. using TopLevelDeclsMap = @@ -9284,6 +9287,43 @@ ReadingKindTracker ReadingKind(Read_Decl, *this); loadDeclUpdateRecords(Update); } + + while (!PendingObjCExtensionIvarRedeclarations.empty()) { + auto ExtensionsPair = PendingObjCExtensionIvarRedeclarations.back().first; + auto DuplicateIvars = + PendingObjCExtensionIvarRedeclarations.back().second; + llvm::DenseSet> NonEquivalentDecls; + StructuralEquivalenceContext Ctx( + ExtensionsPair.first->getASTContext(), + ExtensionsPair.second->getASTContext(), NonEquivalentDecls, + StructuralEquivalenceKind::Default, /*StrictTypeSpelling =*/false, + /*Complain =*/false, + /*ErrorOnTagTypeMismatch =*/true); + if (Ctx.IsEquivalent(ExtensionsPair.first, ExtensionsPair.second)) { + // Merge redeclared ivars with their predecessors. + for (auto IvarPair : DuplicateIvars) { + ObjCIvarDecl *Ivar = IvarPair.first, *PrevIvar = IvarPair.second; + // Change semantic DeclContext but keep the lexical one. + Ivar->setDeclContextsImpl(PrevIvar->getDeclContext(), + Ivar->getLexicalDeclContext(), + getContext()); + getContext().setPrimaryMergedDecl(Ivar, PrevIvar->getCanonicalDecl()); + } + // Invalidate duplicate extension and the cached ivar list. + ExtensionsPair.first->setInvalidDecl(); + ExtensionsPair.second->getClassInterface() + ->getDefinition() + ->setIvarList(nullptr); + } else { + for (auto IvarPair : DuplicateIvars) { + Diag(IvarPair.first->getLocation(), + diag::err_duplicate_ivar_declaration) + << IvarPair.first->getIdentifier(); + Diag(IvarPair.second->getLocation(), diag::note_previous_definition); + } + } + PendingObjCExtensionIvarRedeclarations.pop_back(); + } } // At this point, all update records for loaded decls are in place, so any 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 @@ -36,6 +36,7 @@ #include "clang/AST/Type.h" #include "clang/AST/UnresolvedSet.h" #include "clang/Basic/AttrKinds.h" +#include "clang/Basic/DiagnosticSema.h" #include "clang/Basic/ExceptionSpecificationType.h" #include "clang/Basic/IdentifierTable.h" #include "clang/Basic/LLVM.h" @@ -1232,6 +1233,36 @@ IVD->setNextIvar(nullptr); bool synth = Record.readInt(); IVD->setSynthesize(synth); + + // Check ivar redeclaration. + if (IVD->isInvalidDecl()) + return; + // Don't check ObjCInterfaceDecl as interfaces are named and mismatches can be + // detected in VisitObjCInterfaceDecl. Here we are interested in finding + // redeclarations mostly in extensions. + if (isa(IVD->getDeclContext())) + return; + ObjCInterfaceDecl *CanonIntf = + IVD->getContainingInterface()->getCanonicalDecl(); + IdentifierInfo *II = IVD->getIdentifier(); + ObjCIvarDecl *PrevIvar = CanonIntf->lookupInstanceVariable(II); + if (PrevIvar && PrevIvar != IVD) { + auto *ParentExt = dyn_cast(IVD->getDeclContext()); + auto *PrevParentExt = + dyn_cast(PrevIvar->getDeclContext()); + if (ParentExt && PrevParentExt) { + // Postpone diagnostic as we should merge identical extensions from + // different modules. + Reader + .PendingObjCExtensionIvarRedeclarations[std::make_pair(ParentExt, + PrevParentExt)] + .push_back(std::make_pair(IVD, PrevIvar)); + } else { + Reader.Diag(IVD->getLocation(), diag::err_duplicate_ivar_declaration) + << II; + Reader.Diag(PrevIvar->getLocation(), diag::note_previous_definition); + } + } } void ASTDeclReader::ReadObjCDefinitionData( diff --git a/clang/test/Modules/merge-extension-ivars.m b/clang/test/Modules/merge-extension-ivars.m new file mode 100644 --- /dev/null +++ b/clang/test/Modules/merge-extension-ivars.m @@ -0,0 +1,146 @@ +// UNSUPPORTED: -zos, -aix +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: %clang_cc1 -emit-llvm -o %t/test-compatible-extensions.ll -fobjc-runtime=macosx-10.9 -F%t/Frameworks %t/test-compatible-extensions.m \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -fmodule-name=InterfaceAndExtension +// RUN: FileCheck --input-file=%t/test-compatible-extensions.ll %t/test-compatible-extensions.m + +// RUN: %clang_cc1 -emit-llvm -o %t/test-access-extension-ivar.ll -fobjc-runtime=macosx-10.9 -F%t/Frameworks %t/test-access-extension-ivar.m \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache +// RUN: FileCheck --input-file=%t/test-access-extension-ivar.ll %t/test-access-extension-ivar.m + +// RUN: %clang_cc1 -emit-llvm -o %t/test-synthesized-ivar.ll -fobjc-runtime=macosx-10.9 -F%t/Frameworks %t/test-synthesized-ivar.m \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache +// RUN: FileCheck --input-file=%t/test-synthesized-ivar.ll %t/test-synthesized-ivar.m +// RUN: %clang_cc1 -emit-llvm -o %t/test-synthesized-ivar-extension.ll -fobjc-runtime=macosx-10.9 -F%t/Frameworks %t/test-synthesized-ivar.m \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache \ +// RUN: -DIMPORT_EXTENSION=1 +// RUN: FileCheck --input-file=%t/test-synthesized-ivar-extension.ll %t/test-synthesized-ivar.m + +// Test various scenarios where we can end up with the same-name ivars coming from multiple modules. +// The goal is to avoid duplicate metadata for ivars because it can lead to miscompilations +// with a wrong ivar offset. +// +// See specific .m tests for the details of various scenarios. + +//--- Frameworks/InterfaceAndExtension.framework/Headers/Interface.h +@interface NSObject @end +@interface ObjCInterface : NSObject +@end + +//--- Frameworks/InterfaceAndExtension.framework/Headers/Extension.h +#import +@interface ObjCInterface() { + float ivarInExtension; + int bitfieldIvarInExtension: 3; +} +@end + +//--- Frameworks/InterfaceAndExtension.framework/Headers/InterfaceAndExtension.h +#import +#import + +//--- Frameworks/InterfaceAndExtension.framework/Modules/module.modulemap +framework module InterfaceAndExtension { + umbrella header "InterfaceAndExtension.h" + export * + module * { export * } +} + +//--- Frameworks/Redirecting.framework/Headers/Redirecting.h +#import + +//--- Frameworks/Redirecting.framework/Modules/module.modulemap +framework module Redirecting { + header "Redirecting.h" + export * +} + +//--- test-compatible-extensions.m +// Test adding through deserialization an extension with already declared ivars. + +// First create `ObjCInterface()` extension by parsing corresponding code. +#import +// Now add the same extension through deserialization from the imported module. +#import +@implementation ObjCInterface { + int ivarInImplementation; +} +@end +// CHECK: @"_OBJC_$_INSTANCE_VARIABLES_ObjCInterface" +// CHECK-SAME: [3 x %struct._ivar_t] [%struct._ivar_t { ptr @"OBJC_IVAR_$_ObjCInterface.ivarInExtension", {{.*}} }, %struct._ivar_t { ptr @"OBJC_IVAR_$_ObjCInterface.bitfieldIvarInExtension", {{.*}} }, %struct._ivar_t { ptr @"OBJC_IVAR_$_ObjCInterface.ivarInImplementation", {{.*}} }] + + +//--- Frameworks/WithInlineIvar.framework/Headers/WithInlineIvar.h +#import +@interface ObjCInterface() { +@public + int accessedIvar; +} +@end +static inline void inlinedIvarAccessor(ObjCInterface *obj) { + obj->accessedIvar = 0; +} + +//--- Frameworks/WithInlineIvar.framework/Modules/module.modulemap +framework module WithInlineIvar { + header "WithInlineIvar.h" + export * +} + +//--- test-access-extension-ivar.m +// Test accessing ivars from extensions. +#import +@interface ObjCInterface() { +@public + int accessedIvar; +} +@end +#import +@implementation ObjCInterface +- (void)test { + inlinedIvarAccessor(self); + ivarInExtension = 0; +} +@end +// CHECK: @"_OBJC_$_INSTANCE_VARIABLES_ObjCInterface" +// CHECK-SAME: [3 x %struct._ivar_t] [%struct._ivar_t { ptr @"OBJC_IVAR_$_ObjCInterface.accessedIvar", {{.*}} }, %struct._ivar_t { ptr @"OBJC_IVAR_$_ObjCInterface.ivarInExtension", {{.*}} }, %struct._ivar_t { ptr @"OBJC_IVAR_$_ObjCInterface.bitfieldIvarInExtension", {{.*}} }] + + +//--- Frameworks/WithProperty.framework/Headers/WithProperty.h +@interface NSObject @end +@interface WithProperty: NSObject +@property (assign) int propertyName; +@end + +//--- Frameworks/WithProperty.framework/Modules/module.modulemap +framework module WithProperty { + header "WithProperty.h" + export * +} + +//--- Frameworks/BackingIvarInExtension.framework/Headers/BackingIvarInExtension.h +#import +@interface WithProperty() { + int propertyBackingIvar; +} +@end + +//--- Frameworks/BackingIvarInExtension.framework/Modules/module.modulemap +framework module BackingIvarInExtension { + header "BackingIvarInExtension.h" + export * +} + +//--- test-synthesized-ivar.m +// Test when an ivar is both synthesized and when declared in an extension. +// Behavior with and without extension should be the same. +#import +#ifdef IMPORT_EXTENSION +#import +#endif +@implementation WithProperty +@synthesize propertyName = propertyBackingIvar; +@end +// CHECK: @"_OBJC_$_INSTANCE_VARIABLES_WithProperty" +// CHECK-SAME: [1 x %struct._ivar_t] [%struct._ivar_t { ptr @"OBJC_IVAR_$_WithProperty.propertyBackingIvar", {{.*}} }] diff --git a/clang/test/Modules/redecl-ivars.m b/clang/test/Modules/redecl-ivars.m new file mode 100644 --- /dev/null +++ b/clang/test/Modules/redecl-ivars.m @@ -0,0 +1,166 @@ +// UNSUPPORTED: -zos, -aix +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-mismatch-in-extension.m +// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-mismatch-in-extension.m \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache + +// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-mismatch-in-ivars-number.m +// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-mismatch-in-ivars-number.m \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache + +// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-mismatch-in-methods-protocols.m +// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-mismatch-in-methods-protocols.m \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache + +// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-redecl-in-subclass.m +// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-redecl-in-subclass.m \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache + +// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-redecl-in-implementation.m +// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I%t/include %t/test-redecl-in-implementation.m \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache + +// Same class extensions with the same ivars but from different modules aren't considered +// an error and they are merged together. Test that differences in extensions and/or ivars +// are still reported as errors. + +//--- include/Interfaces.h +@interface NSObject @end +@interface ObjCInterface : NSObject +@end +@interface ObjCInterfaceLevel2 : ObjCInterface +@end + +@protocol Protocol1 @end +@protocol Protocol2 @end + +//--- include/IvarsInExtensions.h +#import +@interface ObjCInterface() { + int ivarName; +} +@end +@interface ObjCInterfaceLevel2() { + int bitfieldIvarName: 3; +} +@end + +//--- include/IvarsInExtensionsWithMethodsProtocols.h +#import +@interface ObjCInterface() { + int methodRelatedIvar; +} +- (void)test; +@end +@interface ObjCInterfaceLevel2() { + int protocolRelatedIvar; +} +@end + +//--- include/IvarInImplementation.h +#import +@implementation ObjCInterface { + int ivarName; +} +@end + +//--- include/module.modulemap +module Interfaces { + header "Interfaces.h" + export * +} +module IvarsInExtensions { + header "IvarsInExtensions.h" + export * +} +module IvarsInExtensionsWithMethodsProtocols { + header "IvarsInExtensionsWithMethodsProtocols.h" + export * +} +module IvarInImplementation { + header "IvarInImplementation.h" + export * +} + + +//--- test-mismatch-in-extension.m +// Different ivars with the same name aren't mergeable and constitute an error. +#import +@interface ObjCInterface() { + float ivarName; // expected-note {{previous definition is here}} +} +@end +@interface ObjCInterfaceLevel2() { + int bitfieldIvarName: 5; // expected-note {{previous definition is here}} +} +@end +#import +// expected-error@IvarsInExtensions.h:* {{instance variable is already declared}} +// expected-error@IvarsInExtensions.h:* {{instance variable is already declared}} +@implementation ObjCInterfaceLevel2 +@end + + +//--- test-mismatch-in-ivars-number.m +// Extensions with different amount of ivars aren't considered to be the same. +#import +@interface ObjCInterface() { + int ivarName; // expected-note {{previous definition is here}} + float anotherIvar; +} +@end +#import +// expected-error@IvarsInExtensions.h:* {{instance variable is already declared}} +@implementation ObjCInterface +@end + + +//--- test-mismatch-in-methods-protocols.m +// Extensions with different methods or protocols aren't considered to be the same. +#import +@interface ObjCInterface() { + int methodRelatedIvar; // expected-note {{previous definition is here}} +} +- (void)differentTest; +@end +@interface ObjCInterfaceLevel2() { + int protocolRelatedIvar; // expected-note {{previous definition is here}} +} +@end +#import +// expected-error@IvarsInExtensionsWithMethodsProtocols.h:* {{instance variable is already declared}} +// expected-error@IvarsInExtensionsWithMethodsProtocols.h:* {{instance variable is already declared}} +@implementation ObjCInterfaceLevel2 +@end + + +//--- test-redecl-in-subclass.m +// Ivar in superclass extension is not added to a subclass, so the ivar with +// the same name in subclass extension is not considered a redeclaration. +// expected-no-diagnostics +#import +@interface ObjCInterfaceLevel2() { + float ivarName; +} +@end +#import +@implementation ObjCInterfaceLevel2 +@end + + +//--- test-redecl-in-implementation.m +// Ivar redeclaration in `@implementation` is always an error and never mergeable. +#import +@interface ObjCInterface() { + int triggerExtensionIvarDeserialization; +} +@end +#import +#if __has_feature(modules) +// expected-error@IvarsInExtensions.h:* {{instance variable is already declared}} +// expected-note@IvarInImplementation.h:* {{previous definition is here}} +#else +// expected-error@IvarInImplementation.h:* {{instance variable is already declared}} +// expected-note@IvarsInExtensions.h:* {{previous definition is here}} +#endif