Skip to content

Commit 7963529

Browse files
jrose-applejrose-apple
authored
Merge pull request #24819 from jrose-apple/when-everyone-is-super-no-one-will-be
[Serialization] Drop a class if the superclass can't be found
2 parents e0658a3 + 96e8d56 commit 7963529

File tree

11 files changed

+239
-35
lines changed

11 files changed

+239
-35
lines changed

‎include/swift/Serialization/ModuleFormat.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
5252
/// describe what change you made. The content of this comment isn't important;
5353
/// it just ensures a conflict if two people change the module format.
5454
/// Don't worry about adhering to the 80-column limit for this line.
55-
const uint16_t SWIFTMODULE_VERSION_MINOR = 491; // mangled class names as vtable keys
55+
const uint16_t SWIFTMODULE_VERSION_MINOR = 493; // dependency types for structs
5656

5757
using DeclIDField = BCFixed<31>;
5858

@@ -948,7 +948,8 @@ namespace decls_block {
948948
GenericEnvironmentIDField, // generic environment
949949
AccessLevelField, // access level
950950
BCVBR<4>, // number of conformances
951-
BCArray<TypeIDField> // inherited types
951+
BCVBR<4>, // number of inherited types
952+
BCArray<TypeIDField> // inherited types, followed by dependency types
952953
// Trailed by the generic parameters (if any), the members record, and
953954
// finally conformance info (if any).
954955
>;
@@ -981,7 +982,8 @@ namespace decls_block {
981982
TypeIDField, // superclass
982983
AccessLevelField, // access level
983984
BCVBR<4>, // number of conformances
984-
BCArray<TypeIDField> // inherited types
985+
BCVBR<4>, // number of inherited types
986+
BCArray<TypeIDField> // inherited types, followed by dependency types
985987
// Trailed by the generic parameters (if any), the members record, and
986988
// finally conformance info (if any).
987989
>;

‎lib/Serialization/Deserialization.cpp

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2488,14 +2488,25 @@ class swift::DeclDeserializer {
24882488
bool isObjC;
24892489
GenericEnvironmentID genericEnvID;
24902490
uint8_t rawAccessLevel;
2491-
unsigned numConformances;
2492-
ArrayRef<uint64_t> rawInheritedIDs;
2491+
unsigned numConformances, numInheritedTypes;
2492+
ArrayRef<uint64_t> rawInheritedAndDependencyIDs;
24932493

24942494
decls_block::StructLayout::readRecord(scratch, nameID, contextID,
24952495
isImplicit, isObjC, genericEnvID,
24962496
rawAccessLevel,
2497-
numConformances,
2498-
rawInheritedIDs);
2497+
numConformances, numInheritedTypes,
2498+
rawInheritedAndDependencyIDs);
2499+
2500+
Identifier name = MF.getIdentifier(nameID);
2501+
2502+
for (TypeID dependencyID :
2503+
rawInheritedAndDependencyIDs.slice(numInheritedTypes)) {
2504+
auto dependency = MF.getTypeChecked(dependencyID);
2505+
if (!dependency) {
2506+
return llvm::make_error<TypeError>(
2507+
name, takeErrorInfo(dependency.takeError()));
2508+
}
2509+
}
24992510

25002511
auto DC = MF.getDeclContext(contextID);
25012512
if (declOrOffset.isComplete())
@@ -2505,10 +2516,8 @@ class swift::DeclDeserializer {
25052516
if (declOrOffset.isComplete())
25062517
return declOrOffset;
25072518

2508-
auto theStruct = MF.createDecl<StructDecl>(SourceLoc(),
2509-
MF.getIdentifier(nameID),
2510-
SourceLoc(), None, genericParams,
2511-
DC);
2519+
auto theStruct = MF.createDecl<StructDecl>(SourceLoc(), name, SourceLoc(),
2520+
None, genericParams, DC);
25122521
declOrOffset = theStruct;
25132522

25142523
// Read the generic environment.
@@ -2528,7 +2537,8 @@ class swift::DeclDeserializer {
25282537

25292538
theStruct->computeType();
25302539

2531-
handleInherited(theStruct, rawInheritedIDs);
2540+
handleInherited(theStruct,
2541+
rawInheritedAndDependencyIDs.slice(0, numInheritedTypes));
25322542

25332543
theStruct->setMemberLoader(&MF, MF.DeclTypeCursor.GetCurrentBitNo());
25342544
skipRecord(MF.DeclTypeCursor, decls_block::MEMBERS);
@@ -3418,15 +3428,27 @@ class swift::DeclDeserializer {
34183428
GenericEnvironmentID genericEnvID;
34193429
TypeID superclassID;
34203430
uint8_t rawAccessLevel;
3421-
unsigned numConformances;
3422-
ArrayRef<uint64_t> rawInheritedIDs;
3431+
unsigned numConformances, numInheritedTypes;
3432+
ArrayRef<uint64_t> rawInheritedAndDependencyIDs;
34233433
decls_block::ClassLayout::readRecord(scratch, nameID, contextID,
34243434
isImplicit, isObjC,
34253435
requiresStoredPropertyInits,
34263436
inheritsSuperclassInitializers,
34273437
genericEnvID, superclassID,
34283438
rawAccessLevel, numConformances,
3429-
rawInheritedIDs);
3439+
numInheritedTypes,
3440+
rawInheritedAndDependencyIDs);
3441+
3442+
Identifier name = MF.getIdentifier(nameID);
3443+
3444+
for (TypeID dependencyID :
3445+
rawInheritedAndDependencyIDs.slice(numInheritedTypes)) {
3446+
auto dependency = MF.getTypeChecked(dependencyID);
3447+
if (!dependency) {
3448+
return llvm::make_error<TypeError>(
3449+
name, takeErrorInfo(dependency.takeError()));
3450+
}
3451+
}
34303452

34313453
auto DC = MF.getDeclContext(contextID);
34323454
if (declOrOffset.isComplete())
@@ -3436,10 +3458,8 @@ class swift::DeclDeserializer {
34363458
if (declOrOffset.isComplete())
34373459
return declOrOffset;
34383460

3439-
auto theClass = MF.createDecl<ClassDecl>(SourceLoc(),
3440-
MF.getIdentifier(nameID),
3441-
SourceLoc(), None, genericParams,
3442-
DC);
3461+
auto theClass = MF.createDecl<ClassDecl>(SourceLoc(), name, SourceLoc(),
3462+
None, genericParams, DC);
34433463
declOrOffset = theClass;
34443464

34453465
MF.configureGenericEnvironment(theClass, genericEnvID);
@@ -3463,7 +3483,8 @@ class swift::DeclDeserializer {
34633483

34643484
theClass->computeType();
34653485

3466-
handleInherited(theClass, rawInheritedIDs);
3486+
handleInherited(theClass,
3487+
rawInheritedAndDependencyIDs.slice(0, numInheritedTypes));
34673488

34683489
theClass->setMemberLoader(&MF, MF.DeclTypeCursor.GetCurrentBitNo());
34693490
theClass->setHasDestructor();

‎lib/Serialization/Serialization.cpp

Lines changed: 64 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2697,22 +2697,39 @@ void Serializer::writeForeignErrorConvention(const ForeignErrorConvention &fec){
26972697
/// - \p decl is declared in an extension of a type that depends on
26982698
/// \p problemContext
26992699
static bool contextDependsOn(const NominalTypeDecl *decl,
2700-
const ModuleDecl *problemModule) {
2701-
return decl->getParentModule() == problemModule;
2700+
const DeclContext *problemContext) {
2701+
SmallPtrSet<const ExtensionDecl *, 8> seenExtensionDCs;
2702+
2703+
const DeclContext *dc = decl;
2704+
do {
2705+
if (dc == problemContext)
2706+
return true;
2707+
2708+
if (auto *extension = dyn_cast<ExtensionDecl>(dc)) {
2709+
if (extension->isChildContextOf(problemContext))
2710+
return true;
2711+
2712+
// Avoid cycles when Left.Nested depends on Right.Nested somehow.
2713+
bool isNewlySeen = seenExtensionDCs.insert(extension).second;
2714+
if (!isNewlySeen)
2715+
break;
2716+
dc = extension->getSelfNominalTypeDecl();
2717+
2718+
} else {
2719+
dc = dc->getParent();
2720+
}
2721+
} while (dc);
2722+
2723+
return false;
27022724
}
27032725

27042726
static void collectDependenciesFromType(llvm::SmallSetVector<Type, 4> &seen,
27052727
Type ty,
2706-
const ModuleDecl *excluding) {
2728+
const DeclContext *excluding) {
27072729
ty.visit([&](Type next) {
27082730
auto *nominal = next->getAnyNominal();
27092731
if (!nominal)
27102732
return;
2711-
// FIXME: Types in the same module are still important for enums. It's
2712-
// possible an enum element has a payload that references a type declaration
2713-
// from the same module that can't be imported (for whatever reason).
2714-
// However, we need a more robust handling of deserialization dependencies
2715-
// that can handle circularities. rdar://problem/32359173
27162733
if (contextDependsOn(nominal, excluding))
27172734
return;
27182735
seen.insert(nominal->getDeclaredInterfaceType());
@@ -2722,7 +2739,7 @@ static void collectDependenciesFromType(llvm::SmallSetVector<Type, 4> &seen,
27222739
static void
27232740
collectDependenciesFromRequirement(llvm::SmallSetVector<Type, 4> &seen,
27242741
const Requirement &req,
2725-
const ModuleDecl *excluding) {
2742+
const DeclContext *excluding) {
27262743
collectDependenciesFromType(seen, req.getFirstType(), excluding);
27272744
if (req.getKind() != RequirementKind::Layout)
27282745
collectDependenciesFromType(seen, req.getSecondType(), excluding);
@@ -3128,12 +3145,20 @@ void Serializer::writeDecl(const Decl *D) {
31283145
auto conformances = theStruct->getLocalConformances(
31293146
ConformanceLookupKind::All, nullptr);
31303147

3131-
SmallVector<TypeID, 4> inheritedTypes;
3148+
SmallVector<TypeID, 4> inheritedAndDependencyTypes;
31323149
for (auto inherited : theStruct->getInherited()) {
31333150
assert(!inherited.getType()->hasArchetype());
3134-
inheritedTypes.push_back(addTypeRef(inherited.getType()));
3151+
inheritedAndDependencyTypes.push_back(addTypeRef(inherited.getType()));
31353152
}
31363153

3154+
llvm::SmallSetVector<Type, 4> dependencyTypes;
3155+
for (Requirement req : theStruct->getGenericRequirements()) {
3156+
collectDependenciesFromRequirement(dependencyTypes, req,
3157+
/*excluding*/nullptr);
3158+
}
3159+
for (Type ty : dependencyTypes)
3160+
inheritedAndDependencyTypes.push_back(addTypeRef(ty));
3161+
31373162
uint8_t rawAccessLevel =
31383163
getRawStableAccessLevel(theStruct->getFormalAccess());
31393164

@@ -3147,7 +3172,8 @@ void Serializer::writeDecl(const Decl *D) {
31473172
theStruct->getGenericEnvironment()),
31483173
rawAccessLevel,
31493174
conformances.size(),
3150-
inheritedTypes);
3175+
theStruct->getInherited().size(),
3176+
inheritedAndDependencyTypes);
31513177

31523178

31533179
writeGenericParams(theStruct->getGenericParams());
@@ -3175,6 +3201,11 @@ void Serializer::writeDecl(const Decl *D) {
31753201
for (const EnumElementDecl *nextElt : theEnum->getAllElements()) {
31763202
if (!nextElt->hasAssociatedValues())
31773203
continue;
3204+
// FIXME: Types in the same module are still important for enums. It's
3205+
// possible an enum element has a payload that references a type
3206+
// declaration from the same module that can't be imported (for whatever
3207+
// reason). However, we need a more robust handling of deserialization
3208+
// dependencies that can handle circularities. rdar://problem/32359173
31783209
collectDependenciesFromType(dependencyTypes,
31793210
nextElt->getArgumentInterfaceType(),
31803211
/*excluding*/theEnum->getParentModule());
@@ -3219,11 +3250,28 @@ void Serializer::writeDecl(const Decl *D) {
32193250
auto conformances = theClass->getLocalConformances(
32203251
ConformanceLookupKind::NonInherited, nullptr);
32213252

3222-
SmallVector<TypeID, 4> inheritedTypes;
3253+
SmallVector<TypeID, 4> inheritedAndDependencyTypes;
32233254
for (auto inherited : theClass->getInherited()) {
32243255
assert(!inherited.getType()->hasArchetype());
3225-
inheritedTypes.push_back(addTypeRef(inherited.getType()));
3256+
inheritedAndDependencyTypes.push_back(addTypeRef(inherited.getType()));
3257+
}
3258+
3259+
llvm::SmallSetVector<Type, 4> dependencyTypes;
3260+
if (theClass->hasSuperclass()) {
3261+
// FIXME: Nested types can still be a problem here: it's possible that (for
3262+
// whatever reason) they won't be able to be deserialized, in which case
3263+
// we'll be in trouble forming the actual superclass type. However, we
3264+
// need a more robust handling of deserialization dependencies that can
3265+
// handle circularities. rdar://problem/50835214
3266+
collectDependenciesFromType(dependencyTypes, theClass->getSuperclass(),
3267+
/*excluding*/theClass);
3268+
}
3269+
for (Requirement req : theClass->getGenericRequirements()) {
3270+
collectDependenciesFromRequirement(dependencyTypes, req,
3271+
/*excluding*/nullptr);
32263272
}
3273+
for (Type ty : dependencyTypes)
3274+
inheritedAndDependencyTypes.push_back(addTypeRef(ty));
32273275

32283276
uint8_t rawAccessLevel =
32293277
getRawStableAccessLevel(theClass->getFormalAccess());
@@ -3245,7 +3293,8 @@ void Serializer::writeDecl(const Decl *D) {
32453293
addTypeRef(theClass->getSuperclass()),
32463294
rawAccessLevel,
32473295
conformances.size(),
3248-
inheritedTypes);
3296+
theClass->getInherited().size(),
3297+
inheritedAndDependencyTypes);
32493298

32503299
writeGenericParams(theClass->getGenericParams());
32513300
writeMembers(id, theClass->getMembers(), true);

‎test/Serialization/Recovery/Inputs/custom-modules/SuperclassObjC.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
@interface Root
2+
- (instancetype)init;
3+
@end
4+
5+
#if !BAD
6+
@interface DisappearingSuperclass : Root
7+
@end
8+
#else
9+
#endif

‎test/Serialization/Recovery/Inputs/custom-modules/module.modulemap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ module IndirectlyImported {
99
module Overrides { header "Overrides.h" }
1010
module ProtocolInheritance { header "ProtocolInheritance.h" }
1111
module RenameAcrossVersions { header "RenameAcrossVersions.h" }
12+
module SuperclassObjC { header "SuperclassObjC.h" }
1213
module Typedefs { header "Typedefs.h" }
1314
module TypeRemovalObjC { header "TypeRemovalObjC.h" }
1415
module Types { header "Types.h" }

‎test/Serialization/Recovery/superclass.swift

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// RUN: %empty-directory(%t)
2+
// 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
3+
4+
// 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
5+
6+
// 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
7+
8+
// RUN: %target-swift-frontend -typecheck -enable-objc-interop -I %t -I %S/Inputs/custom-modules -Xcc -DBAD -DTEST %s -verify
9+
10+
#if TEST
11+
12+
import Lib
13+
14+
func use(_: C2_CRTPish) {}
15+
func use(_: C3_NestingCRTPish) {}
16+
func use(_: C4_ExtensionNestingCRTPish) {}
17+
18+
// FIXME: Better to import the class and make it unavailable.
19+
func use(_: A1_Sub) {} // expected-error {{use of undeclared type 'A1_Sub'}}
20+
func use(_: A2_Grandchild) {} // expected-error {{use of undeclared type 'A2_Grandchild'}}
21+
22+
#else // TEST
23+
24+
import SuperclassObjC
25+
26+
// CHECK-LABEL: class A1_Sub : DisappearingSuperclass {
27+
// CHECK-RECOVERY-NOT: class A1_Sub
28+
public class A1_Sub: DisappearingSuperclass {}
29+
// CHECK-LABEL: class A2_Grandchild : A1_Sub {
30+
// CHECK-RECOVERY-NOT: class A2_Grandchild
31+
public class A2_Grandchild: A1_Sub {}
32+
33+
// CHECK-LABEL: class B1_ConstrainedGeneric<T> where T : DisappearingSuperclass {
34+
// CHECK-RECOVERY-NOT: class B1_ConstrainedGeneric
35+
public class B1_ConstrainedGeneric<T> where T: DisappearingSuperclass {}
36+
37+
// CHECK-LABEL: struct B2_ConstrainedGeneric<T> where T : DisappearingSuperclass {
38+
// CHECK-RECOVERY-NOT: struct B2_ConstrainedGeneric
39+
public struct B2_ConstrainedGeneric<T> where T: DisappearingSuperclass {}
40+
41+
// CHECK-ALWAYS-LABEL: class C1_GenericBase<T> {
42+
public class C1_GenericBase<T> {}
43+
44+
// CHECK-ALWAYS-LABEL: class C2_CRTPish : C1_GenericBase<C2_CRTPish> {
45+
public class C2_CRTPish: C1_GenericBase<C2_CRTPish> {}
46+
// CHECK-ALWAYS-LABEL: class C3_NestingCRTPish : C1_GenericBase<C3_NestingCRTPish.Nested> {
47+
public class C3_NestingCRTPish: C1_GenericBase<C3_NestingCRTPish.Nested> {
48+
public struct Nested {}
49+
}
50+
// CHECK-ALWAYS-LABEL: class C4_ExtensionNestingCRTPish : C1_GenericBase<C4_ExtensionNestingCRTPish.Nested> {
51+
public class C4_ExtensionNestingCRTPish: C1_GenericBase<C4_ExtensionNestingCRTPish.Nested> {}
52+
extension C4_ExtensionNestingCRTPish {
53+
public struct Nested {}
54+
}
55+
56+
#endif // TEST

‎test/Serialization/Recovery/typedefs-in-enums.swift

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,4 +110,31 @@ public func producesOkayEnum() -> OkayEnum { return .noPayload }
110110
// CHECK-LABEL: func producesOkayEnum() -> OkayEnum
111111
// CHECK-RECOVERY-LABEL: func producesOkayEnum() -> OkayEnum
112112

113+
114+
extension Int /* or any imported type, really */ {
115+
public enum OkayEnumWithSelfRefs {
116+
public struct Nested {}
117+
indirect case selfRef(OkayEnumWithSelfRefs)
118+
case nested(Nested)
119+
}
120+
}
121+
// CHECK-LABEL: extension Int {
122+
// CHECK-NEXT: enum OkayEnumWithSelfRefs {
123+
// CHECK-NEXT: struct Nested {
124+
// CHECK-NEXT: init()
125+
// CHECK-NEXT: }
126+
// CHECK-NEXT: indirect case selfRef(Int.OkayEnumWithSelfRefs)
127+
// CHECK-NEXT: case nested(Int.OkayEnumWithSelfRefs.Nested)
128+
// CHECK-NEXT: }
129+
// CHECK-NEXT: }
130+
// CHECK-RECOVERY-LABEL: extension Int {
131+
// CHECK-RECOVERY-NEXT: enum OkayEnumWithSelfRefs {
132+
// CHECK-RECOVERY-NEXT: struct Nested {
133+
// CHECK-RECOVERY-NEXT: init()
134+
// CHECK-RECOVERY-NEXT: }
135+
// CHECK-RECOVERY-NEXT: indirect case selfRef(Int.OkayEnumWithSelfRefs)
136+
// CHECK-RECOVERY-NEXT: case nested(Int.OkayEnumWithSelfRefs.Nested)
137+
// CHECK-RECOVERY-NEXT: }
138+
// CHECK-RECOVERY-NEXT: }
139+
113140
#endif // TEST

‎validation-test/Serialization/Inputs/custom-modules/SuperclassObjC.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
@interface Root
2+
- (instancetype)init;
3+
@end
4+
5+
#if !BAD
6+
@interface DisappearingSuperclass : Root
7+
@end
8+
#else
9+
#endif

‎validation-test/Serialization/Inputs/custom-modules/module.modulemap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
module rdar40899824Helper { header "rdar40899824Helper.h" }
2+
module SuperclassObjC { header "SuperclassObjC.h" }

0 commit comments

Comments
 (0)