diff --git a/include/swift/Serialization/ModuleFormat.h b/include/swift/Serialization/ModuleFormat.h index 90cfd26dd73ca..65f32f7c0ae52 100644 --- a/include/swift/Serialization/ModuleFormat.h +++ b/include/swift/Serialization/ModuleFormat.h @@ -52,7 +52,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0; /// describe what change you made. The content of this comment isn't important; /// it just ensures a conflict if two people change the module format. /// Don't worry about adhering to the 80-column limit for this line. -const uint16_t SWIFTMODULE_VERSION_MINOR = 491; // mangled class names as vtable keys +const uint16_t SWIFTMODULE_VERSION_MINOR = 493; // dependency types for structs using DeclIDField = BCFixed<31>; @@ -948,7 +948,8 @@ namespace decls_block { GenericEnvironmentIDField, // generic environment AccessLevelField, // access level BCVBR<4>, // number of conformances - BCArray // inherited types + BCVBR<4>, // number of inherited types + BCArray // inherited types, followed by dependency types // Trailed by the generic parameters (if any), the members record, and // finally conformance info (if any). >; @@ -981,7 +982,8 @@ namespace decls_block { TypeIDField, // superclass AccessLevelField, // access level BCVBR<4>, // number of conformances - BCArray // inherited types + BCVBR<4>, // number of inherited types + BCArray // inherited types, followed by dependency types // Trailed by the generic parameters (if any), the members record, and // finally conformance info (if any). >; diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index c5b010530caa4..0eca879073728 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -2488,14 +2488,25 @@ class swift::DeclDeserializer { bool isObjC; GenericEnvironmentID genericEnvID; uint8_t rawAccessLevel; - unsigned numConformances; - ArrayRef rawInheritedIDs; + unsigned numConformances, numInheritedTypes; + ArrayRef rawInheritedAndDependencyIDs; decls_block::StructLayout::readRecord(scratch, nameID, contextID, isImplicit, isObjC, genericEnvID, rawAccessLevel, - numConformances, - rawInheritedIDs); + numConformances, numInheritedTypes, + rawInheritedAndDependencyIDs); + + Identifier name = MF.getIdentifier(nameID); + + for (TypeID dependencyID : + rawInheritedAndDependencyIDs.slice(numInheritedTypes)) { + auto dependency = MF.getTypeChecked(dependencyID); + if (!dependency) { + return llvm::make_error( + name, takeErrorInfo(dependency.takeError())); + } + } auto DC = MF.getDeclContext(contextID); if (declOrOffset.isComplete()) @@ -2505,10 +2516,8 @@ class swift::DeclDeserializer { if (declOrOffset.isComplete()) return declOrOffset; - auto theStruct = MF.createDecl(SourceLoc(), - MF.getIdentifier(nameID), - SourceLoc(), None, genericParams, - DC); + auto theStruct = MF.createDecl(SourceLoc(), name, SourceLoc(), + None, genericParams, DC); declOrOffset = theStruct; // Read the generic environment. @@ -2528,7 +2537,8 @@ class swift::DeclDeserializer { theStruct->computeType(); - handleInherited(theStruct, rawInheritedIDs); + handleInherited(theStruct, + rawInheritedAndDependencyIDs.slice(0, numInheritedTypes)); theStruct->setMemberLoader(&MF, MF.DeclTypeCursor.GetCurrentBitNo()); skipRecord(MF.DeclTypeCursor, decls_block::MEMBERS); @@ -3418,15 +3428,27 @@ class swift::DeclDeserializer { GenericEnvironmentID genericEnvID; TypeID superclassID; uint8_t rawAccessLevel; - unsigned numConformances; - ArrayRef rawInheritedIDs; + unsigned numConformances, numInheritedTypes; + ArrayRef rawInheritedAndDependencyIDs; decls_block::ClassLayout::readRecord(scratch, nameID, contextID, isImplicit, isObjC, requiresStoredPropertyInits, inheritsSuperclassInitializers, genericEnvID, superclassID, rawAccessLevel, numConformances, - rawInheritedIDs); + numInheritedTypes, + rawInheritedAndDependencyIDs); + + Identifier name = MF.getIdentifier(nameID); + + for (TypeID dependencyID : + rawInheritedAndDependencyIDs.slice(numInheritedTypes)) { + auto dependency = MF.getTypeChecked(dependencyID); + if (!dependency) { + return llvm::make_error( + name, takeErrorInfo(dependency.takeError())); + } + } auto DC = MF.getDeclContext(contextID); if (declOrOffset.isComplete()) @@ -3436,10 +3458,8 @@ class swift::DeclDeserializer { if (declOrOffset.isComplete()) return declOrOffset; - auto theClass = MF.createDecl(SourceLoc(), - MF.getIdentifier(nameID), - SourceLoc(), None, genericParams, - DC); + auto theClass = MF.createDecl(SourceLoc(), name, SourceLoc(), + None, genericParams, DC); declOrOffset = theClass; MF.configureGenericEnvironment(theClass, genericEnvID); @@ -3463,7 +3483,8 @@ class swift::DeclDeserializer { theClass->computeType(); - handleInherited(theClass, rawInheritedIDs); + handleInherited(theClass, + rawInheritedAndDependencyIDs.slice(0, numInheritedTypes)); theClass->setMemberLoader(&MF, MF.DeclTypeCursor.GetCurrentBitNo()); theClass->setHasDestructor(); diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp index 7cf75c33003e5..cdddb4da3d0ed 100644 --- a/lib/Serialization/Serialization.cpp +++ b/lib/Serialization/Serialization.cpp @@ -2697,22 +2697,39 @@ void Serializer::writeForeignErrorConvention(const ForeignErrorConvention &fec){ /// - \p decl is declared in an extension of a type that depends on /// \p problemContext static bool contextDependsOn(const NominalTypeDecl *decl, - const ModuleDecl *problemModule) { - return decl->getParentModule() == problemModule; + const DeclContext *problemContext) { + SmallPtrSet seenExtensionDCs; + + const DeclContext *dc = decl; + do { + if (dc == problemContext) + return true; + + if (auto *extension = dyn_cast(dc)) { + if (extension->isChildContextOf(problemContext)) + return true; + + // Avoid cycles when Left.Nested depends on Right.Nested somehow. + bool isNewlySeen = seenExtensionDCs.insert(extension).second; + if (!isNewlySeen) + break; + dc = extension->getSelfNominalTypeDecl(); + + } else { + dc = dc->getParent(); + } + } while (dc); + + return false; } static void collectDependenciesFromType(llvm::SmallSetVector &seen, Type ty, - const ModuleDecl *excluding) { + const DeclContext *excluding) { ty.visit([&](Type next) { auto *nominal = next->getAnyNominal(); if (!nominal) return; - // FIXME: Types in the same module are still important for enums. It's - // possible an enum element has a payload that references a type declaration - // from the same module that can't be imported (for whatever reason). - // However, we need a more robust handling of deserialization dependencies - // that can handle circularities. rdar://problem/32359173 if (contextDependsOn(nominal, excluding)) return; seen.insert(nominal->getDeclaredInterfaceType()); @@ -2722,7 +2739,7 @@ static void collectDependenciesFromType(llvm::SmallSetVector &seen, static void collectDependenciesFromRequirement(llvm::SmallSetVector &seen, const Requirement &req, - const ModuleDecl *excluding) { + const DeclContext *excluding) { collectDependenciesFromType(seen, req.getFirstType(), excluding); if (req.getKind() != RequirementKind::Layout) collectDependenciesFromType(seen, req.getSecondType(), excluding); @@ -3128,12 +3145,20 @@ void Serializer::writeDecl(const Decl *D) { auto conformances = theStruct->getLocalConformances( ConformanceLookupKind::All, nullptr); - SmallVector inheritedTypes; + SmallVector inheritedAndDependencyTypes; for (auto inherited : theStruct->getInherited()) { assert(!inherited.getType()->hasArchetype()); - inheritedTypes.push_back(addTypeRef(inherited.getType())); + inheritedAndDependencyTypes.push_back(addTypeRef(inherited.getType())); } + llvm::SmallSetVector dependencyTypes; + for (Requirement req : theStruct->getGenericRequirements()) { + collectDependenciesFromRequirement(dependencyTypes, req, + /*excluding*/nullptr); + } + for (Type ty : dependencyTypes) + inheritedAndDependencyTypes.push_back(addTypeRef(ty)); + uint8_t rawAccessLevel = getRawStableAccessLevel(theStruct->getFormalAccess()); @@ -3147,7 +3172,8 @@ void Serializer::writeDecl(const Decl *D) { theStruct->getGenericEnvironment()), rawAccessLevel, conformances.size(), - inheritedTypes); + theStruct->getInherited().size(), + inheritedAndDependencyTypes); writeGenericParams(theStruct->getGenericParams()); @@ -3175,6 +3201,11 @@ void Serializer::writeDecl(const Decl *D) { for (const EnumElementDecl *nextElt : theEnum->getAllElements()) { if (!nextElt->hasAssociatedValues()) continue; + // FIXME: Types in the same module are still important for enums. It's + // possible an enum element has a payload that references a type + // declaration from the same module that can't be imported (for whatever + // reason). However, we need a more robust handling of deserialization + // dependencies that can handle circularities. rdar://problem/32359173 collectDependenciesFromType(dependencyTypes, nextElt->getArgumentInterfaceType(), /*excluding*/theEnum->getParentModule()); @@ -3219,11 +3250,28 @@ void Serializer::writeDecl(const Decl *D) { auto conformances = theClass->getLocalConformances( ConformanceLookupKind::NonInherited, nullptr); - SmallVector inheritedTypes; + SmallVector inheritedAndDependencyTypes; for (auto inherited : theClass->getInherited()) { assert(!inherited.getType()->hasArchetype()); - inheritedTypes.push_back(addTypeRef(inherited.getType())); + inheritedAndDependencyTypes.push_back(addTypeRef(inherited.getType())); + } + + llvm::SmallSetVector dependencyTypes; + if (theClass->hasSuperclass()) { + // FIXME: Nested types can still be a problem here: it's possible that (for + // whatever reason) they won't be able to be deserialized, in which case + // we'll be in trouble forming the actual superclass type. However, we + // need a more robust handling of deserialization dependencies that can + // handle circularities. rdar://problem/50835214 + collectDependenciesFromType(dependencyTypes, theClass->getSuperclass(), + /*excluding*/theClass); + } + for (Requirement req : theClass->getGenericRequirements()) { + collectDependenciesFromRequirement(dependencyTypes, req, + /*excluding*/nullptr); } + for (Type ty : dependencyTypes) + inheritedAndDependencyTypes.push_back(addTypeRef(ty)); uint8_t rawAccessLevel = getRawStableAccessLevel(theClass->getFormalAccess()); @@ -3245,7 +3293,8 @@ void Serializer::writeDecl(const Decl *D) { addTypeRef(theClass->getSuperclass()), rawAccessLevel, conformances.size(), - inheritedTypes); + theClass->getInherited().size(), + inheritedAndDependencyTypes); writeGenericParams(theClass->getGenericParams()); writeMembers(id, theClass->getMembers(), true); diff --git a/test/Serialization/Recovery/Inputs/custom-modules/SuperclassObjC.h b/test/Serialization/Recovery/Inputs/custom-modules/SuperclassObjC.h new file mode 100644 index 0000000000000..01b5f395eaf3d --- /dev/null +++ b/test/Serialization/Recovery/Inputs/custom-modules/SuperclassObjC.h @@ -0,0 +1,9 @@ +@interface Root +- (instancetype)init; +@end + +#if !BAD +@interface DisappearingSuperclass : Root +@end +#else +#endif diff --git a/test/Serialization/Recovery/Inputs/custom-modules/module.modulemap b/test/Serialization/Recovery/Inputs/custom-modules/module.modulemap index eb1a2968d497e..203ab78816cd6 100644 --- a/test/Serialization/Recovery/Inputs/custom-modules/module.modulemap +++ b/test/Serialization/Recovery/Inputs/custom-modules/module.modulemap @@ -9,6 +9,7 @@ module IndirectlyImported { module Overrides { header "Overrides.h" } module ProtocolInheritance { header "ProtocolInheritance.h" } module RenameAcrossVersions { header "RenameAcrossVersions.h" } +module SuperclassObjC { header "SuperclassObjC.h" } module Typedefs { header "Typedefs.h" } module TypeRemovalObjC { header "TypeRemovalObjC.h" } module Types { header "Types.h" } diff --git a/test/Serialization/Recovery/superclass.swift b/test/Serialization/Recovery/superclass.swift new file mode 100644 index 0000000000000..759ae6f018d92 --- /dev/null +++ b/test/Serialization/Recovery/superclass.swift @@ -0,0 +1,56 @@ +// RUN: %empty-directory(%t) +// RUN: %target-swift-frontend -emit-sil -enable-objc-interop -o - -emit-module-path %t/Lib.swiftmodule -module-name Lib -I %S/Inputs/custom-modules %s > /dev/null + +// RUN: %target-swift-ide-test -enable-objc-interop -source-filename=x -print-module -module-to-print Lib -I %t -I %S/Inputs/custom-modules | %FileCheck -check-prefix CHECK -check-prefix CHECK-ALWAYS %s + +// RUN: %target-swift-ide-test -enable-objc-interop -source-filename=x -print-module -module-to-print Lib -I %t -I %S/Inputs/custom-modules -Xcc -DBAD | %FileCheck -check-prefix CHECK-RECOVERY -check-prefix CHECK-ALWAYS %s + +// RUN: %target-swift-frontend -typecheck -enable-objc-interop -I %t -I %S/Inputs/custom-modules -Xcc -DBAD -DTEST %s -verify + +#if TEST + +import Lib + +func use(_: C2_CRTPish) {} +func use(_: C3_NestingCRTPish) {} +func use(_: C4_ExtensionNestingCRTPish) {} + +// FIXME: Better to import the class and make it unavailable. +func use(_: A1_Sub) {} // expected-error {{use of undeclared type 'A1_Sub'}} +func use(_: A2_Grandchild) {} // expected-error {{use of undeclared type 'A2_Grandchild'}} + +#else // TEST + +import SuperclassObjC + +// CHECK-LABEL: class A1_Sub : DisappearingSuperclass { +// CHECK-RECOVERY-NOT: class A1_Sub +public class A1_Sub: DisappearingSuperclass {} +// CHECK-LABEL: class A2_Grandchild : A1_Sub { +// CHECK-RECOVERY-NOT: class A2_Grandchild +public class A2_Grandchild: A1_Sub {} + +// CHECK-LABEL: class B1_ConstrainedGeneric where T : DisappearingSuperclass { +// CHECK-RECOVERY-NOT: class B1_ConstrainedGeneric +public class B1_ConstrainedGeneric where T: DisappearingSuperclass {} + +// CHECK-LABEL: struct B2_ConstrainedGeneric where T : DisappearingSuperclass { +// CHECK-RECOVERY-NOT: struct B2_ConstrainedGeneric +public struct B2_ConstrainedGeneric where T: DisappearingSuperclass {} + +// CHECK-ALWAYS-LABEL: class C1_GenericBase { +public class C1_GenericBase {} + +// CHECK-ALWAYS-LABEL: class C2_CRTPish : C1_GenericBase { +public class C2_CRTPish: C1_GenericBase {} +// CHECK-ALWAYS-LABEL: class C3_NestingCRTPish : C1_GenericBase { +public class C3_NestingCRTPish: C1_GenericBase { + public struct Nested {} +} +// CHECK-ALWAYS-LABEL: class C4_ExtensionNestingCRTPish : C1_GenericBase { +public class C4_ExtensionNestingCRTPish: C1_GenericBase {} +extension C4_ExtensionNestingCRTPish { + public struct Nested {} +} + +#endif // TEST diff --git a/test/Serialization/Recovery/typedefs-in-enums.swift b/test/Serialization/Recovery/typedefs-in-enums.swift index f1514715fdeb0..233a9a8d266d0 100644 --- a/test/Serialization/Recovery/typedefs-in-enums.swift +++ b/test/Serialization/Recovery/typedefs-in-enums.swift @@ -110,4 +110,31 @@ public func producesOkayEnum() -> OkayEnum { return .noPayload } // CHECK-LABEL: func producesOkayEnum() -> OkayEnum // CHECK-RECOVERY-LABEL: func producesOkayEnum() -> OkayEnum + +extension Int /* or any imported type, really */ { + public enum OkayEnumWithSelfRefs { + public struct Nested {} + indirect case selfRef(OkayEnumWithSelfRefs) + case nested(Nested) + } +} +// CHECK-LABEL: extension Int { +// CHECK-NEXT: enum OkayEnumWithSelfRefs { +// CHECK-NEXT: struct Nested { +// CHECK-NEXT: init() +// CHECK-NEXT: } +// CHECK-NEXT: indirect case selfRef(Int.OkayEnumWithSelfRefs) +// CHECK-NEXT: case nested(Int.OkayEnumWithSelfRefs.Nested) +// CHECK-NEXT: } +// CHECK-NEXT: } +// CHECK-RECOVERY-LABEL: extension Int { +// CHECK-RECOVERY-NEXT: enum OkayEnumWithSelfRefs { +// CHECK-RECOVERY-NEXT: struct Nested { +// CHECK-RECOVERY-NEXT: init() +// CHECK-RECOVERY-NEXT: } +// CHECK-RECOVERY-NEXT: indirect case selfRef(Int.OkayEnumWithSelfRefs) +// CHECK-RECOVERY-NEXT: case nested(Int.OkayEnumWithSelfRefs.Nested) +// CHECK-RECOVERY-NEXT: } +// CHECK-RECOVERY-NEXT: } + #endif // TEST diff --git a/validation-test/Serialization/Inputs/custom-modules/SuperclassObjC.h b/validation-test/Serialization/Inputs/custom-modules/SuperclassObjC.h new file mode 100644 index 0000000000000..01b5f395eaf3d --- /dev/null +++ b/validation-test/Serialization/Inputs/custom-modules/SuperclassObjC.h @@ -0,0 +1,9 @@ +@interface Root +- (instancetype)init; +@end + +#if !BAD +@interface DisappearingSuperclass : Root +@end +#else +#endif diff --git a/validation-test/Serialization/Inputs/custom-modules/module.modulemap b/validation-test/Serialization/Inputs/custom-modules/module.modulemap index b8ef16f10817d..8eacf94ed3cd6 100644 --- a/validation-test/Serialization/Inputs/custom-modules/module.modulemap +++ b/validation-test/Serialization/Inputs/custom-modules/module.modulemap @@ -1 +1,2 @@ module rdar40899824Helper { header "rdar40899824Helper.h" } +module SuperclassObjC { header "SuperclassObjC.h" } diff --git a/validation-test/Serialization/crash-superclass-dependency-cycle.swift b/validation-test/Serialization/crash-superclass-dependency-cycle.swift new file mode 100644 index 0000000000000..e0ff3ace7190c --- /dev/null +++ b/validation-test/Serialization/crash-superclass-dependency-cycle.swift @@ -0,0 +1,13 @@ +// RUN: %empty-directory(%t) +// RUN: %target-swift-frontend -emit-module -o %t/Lib.swiftmodule -I %S/Inputs/custom-modules %s + +// FIXME: We need a way to handle the cyclic dependency here. +// rdar://problem/50835214 +// RUN: not --crash %target-swift-ide-test -source-filename=x -print-module -module-to-print Lib -I %t -I %S/Inputs/custom-modules -Xcc -DBAD + +public class GenericBase {} +public class Sub: GenericBase { +} +public class Grandchild: Sub { + public class Nested {} +} diff --git a/validation-test/Serialization/crash-superclass-dependency-removal.swift b/validation-test/Serialization/crash-superclass-dependency-removal.swift new file mode 100644 index 0000000000000..2a35d0165bb74 --- /dev/null +++ b/validation-test/Serialization/crash-superclass-dependency-removal.swift @@ -0,0 +1,16 @@ +// RUN: %empty-directory(%t) +// RUN: %target-swift-frontend -emit-module -o %t/Lib.swiftmodule -enable-objc-interop -I %S/Inputs/custom-modules %s +// RUN: %target-swift-ide-test -source-filename=x -print-module -module-to-print Lib -enable-objc-interop -I %t -I %S/Inputs/custom-modules | %FileCheck %s + +// FIXME: We need a way to handle the disappearing superclass in a position we +// can't track easily from the containing class. rdar://problem/50835214 +// RUN: not --crash %target-swift-ide-test -source-filename=x -print-module -module-to-print Lib -enable-objc-interop -I %t -I %S/Inputs/custom-modules -Xcc -DBAD + +import SuperclassObjC + +public class GenericBase {} + +// CHECK: class Sub : GenericBase { +public class Sub: GenericBase { + public class Nested: DisappearingSuperclass {} +}