Skip to content

Commit 29cd2a9

Browse files
jrose-applejrose-apple
authored
Merge pull request #24819 from jrose-apple/when-everyone-is-super-no-one-will-be (#24915)
[Serialization] Drop a class if the superclass can't be found (cherry picked from commit 7963529) rdar://problem/50125674
1 parent 9d6ca0c commit 29cd2a9

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 = 489; // mangled class names as vtable keys
55+
const uint16_t SWIFTMODULE_VERSION_MINOR = 490; // 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
@@ -2497,14 +2497,25 @@ class swift::DeclDeserializer {
24972497
bool isObjC;
24982498
GenericEnvironmentID genericEnvID;
24992499
uint8_t rawAccessLevel;
2500-
unsigned numConformances;
2501-
ArrayRef<uint64_t> rawInheritedIDs;
2500+
unsigned numConformances, numInheritedTypes;
2501+
ArrayRef<uint64_t> rawInheritedAndDependencyIDs;
25022502

25032503
decls_block::StructLayout::readRecord(scratch, nameID, contextID,
25042504
isImplicit, isObjC, genericEnvID,
25052505
rawAccessLevel,
2506-
numConformances,
2507-
rawInheritedIDs);
2506+
numConformances, numInheritedTypes,
2507+
rawInheritedAndDependencyIDs);
2508+
2509+
Identifier name = MF.getIdentifier(nameID);
2510+
2511+
for (TypeID dependencyID :
2512+
rawInheritedAndDependencyIDs.slice(numInheritedTypes)) {
2513+
auto dependency = MF.getTypeChecked(dependencyID);
2514+
if (!dependency) {
2515+
return llvm::make_error<TypeError>(
2516+
name, takeErrorInfo(dependency.takeError()));
2517+
}
2518+
}
25082519

25092520
auto DC = MF.getDeclContext(contextID);
25102521
if (declOrOffset.isComplete())
@@ -2514,10 +2525,8 @@ class swift::DeclDeserializer {
25142525
if (declOrOffset.isComplete())
25152526
return declOrOffset;
25162527

2517-
auto theStruct = MF.createDecl<StructDecl>(SourceLoc(),
2518-
MF.getIdentifier(nameID),
2519-
SourceLoc(), None, genericParams,
2520-
DC);
2528+
auto theStruct = MF.createDecl<StructDecl>(SourceLoc(), name, SourceLoc(),
2529+
None, genericParams, DC);
25212530
declOrOffset = theStruct;
25222531

25232532
// Read the generic environment.
@@ -2537,7 +2546,8 @@ class swift::DeclDeserializer {
25372546

25382547
theStruct->computeType();
25392548

2540-
handleInherited(theStruct, rawInheritedIDs);
2549+
handleInherited(theStruct,
2550+
rawInheritedAndDependencyIDs.slice(0, numInheritedTypes));
25412551

25422552
theStruct->setMemberLoader(&MF, MF.DeclTypeCursor.GetCurrentBitNo());
25432553
skipRecord(MF.DeclTypeCursor, decls_block::MEMBERS);
@@ -3424,15 +3434,27 @@ class swift::DeclDeserializer {
34243434
GenericEnvironmentID genericEnvID;
34253435
TypeID superclassID;
34263436
uint8_t rawAccessLevel;
3427-
unsigned numConformances;
3428-
ArrayRef<uint64_t> rawInheritedIDs;
3437+
unsigned numConformances, numInheritedTypes;
3438+
ArrayRef<uint64_t> rawInheritedAndDependencyIDs;
34293439
decls_block::ClassLayout::readRecord(scratch, nameID, contextID,
34303440
isImplicit, isObjC,
34313441
requiresStoredPropertyInits,
34323442
inheritsSuperclassInitializers,
34333443
genericEnvID, superclassID,
34343444
rawAccessLevel, numConformances,
3435-
rawInheritedIDs);
3445+
numInheritedTypes,
3446+
rawInheritedAndDependencyIDs);
3447+
3448+
Identifier name = MF.getIdentifier(nameID);
3449+
3450+
for (TypeID dependencyID :
3451+
rawInheritedAndDependencyIDs.slice(numInheritedTypes)) {
3452+
auto dependency = MF.getTypeChecked(dependencyID);
3453+
if (!dependency) {
3454+
return llvm::make_error<TypeError>(
3455+
name, takeErrorInfo(dependency.takeError()));
3456+
}
3457+
}
34363458

34373459
auto DC = MF.getDeclContext(contextID);
34383460
if (declOrOffset.isComplete())
@@ -3442,10 +3464,8 @@ class swift::DeclDeserializer {
34423464
if (declOrOffset.isComplete())
34433465
return declOrOffset;
34443466

3445-
auto theClass = MF.createDecl<ClassDecl>(SourceLoc(),
3446-
MF.getIdentifier(nameID),
3447-
SourceLoc(), None, genericParams,
3448-
DC);
3467+
auto theClass = MF.createDecl<ClassDecl>(SourceLoc(), name, SourceLoc(),
3468+
None, genericParams, DC);
34493469
declOrOffset = theClass;
34503470

34513471
MF.configureGenericEnvironment(theClass, genericEnvID);
@@ -3469,7 +3489,8 @@ class swift::DeclDeserializer {
34693489

34703490
theClass->computeType();
34713491

3472-
handleInherited(theClass, rawInheritedIDs);
3492+
handleInherited(theClass,
3493+
rawInheritedAndDependencyIDs.slice(0, numInheritedTypes));
34733494

34743495
theClass->setMemberLoader(&MF, MF.DeclTypeCursor.GetCurrentBitNo());
34753496
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);
@@ -3123,12 +3140,20 @@ void Serializer::writeDecl(const Decl *D) {
31233140
auto conformances = theStruct->getLocalConformances(
31243141
ConformanceLookupKind::All, nullptr);
31253142

3126-
SmallVector<TypeID, 4> inheritedTypes;
3143+
SmallVector<TypeID, 4> inheritedAndDependencyTypes;
31273144
for (auto inherited : theStruct->getInherited()) {
31283145
assert(!inherited.getType()->hasArchetype());
3129-
inheritedTypes.push_back(addTypeRef(inherited.getType()));
3146+
inheritedAndDependencyTypes.push_back(addTypeRef(inherited.getType()));
31303147
}
31313148

3149+
llvm::SmallSetVector<Type, 4> dependencyTypes;
3150+
for (Requirement req : theStruct->getGenericRequirements()) {
3151+
collectDependenciesFromRequirement(dependencyTypes, req,
3152+
/*excluding*/nullptr);
3153+
}
3154+
for (Type ty : dependencyTypes)
3155+
inheritedAndDependencyTypes.push_back(addTypeRef(ty));
3156+
31323157
uint8_t rawAccessLevel =
31333158
getRawStableAccessLevel(theStruct->getFormalAccess());
31343159

@@ -3142,7 +3167,8 @@ void Serializer::writeDecl(const Decl *D) {
31423167
theStruct->getGenericEnvironment()),
31433168
rawAccessLevel,
31443169
conformances.size(),
3145-
inheritedTypes);
3170+
theStruct->getInherited().size(),
3171+
inheritedAndDependencyTypes);
31463172

31473173

31483174
writeGenericParams(theStruct->getGenericParams());
@@ -3170,6 +3196,11 @@ void Serializer::writeDecl(const Decl *D) {
31703196
for (const EnumElementDecl *nextElt : theEnum->getAllElements()) {
31713197
if (!nextElt->hasAssociatedValues())
31723198
continue;
3199+
// FIXME: Types in the same module are still important for enums. It's
3200+
// possible an enum element has a payload that references a type
3201+
// declaration from the same module that can't be imported (for whatever
3202+
// reason). However, we need a more robust handling of deserialization
3203+
// dependencies that can handle circularities. rdar://problem/32359173
31733204
collectDependenciesFromType(dependencyTypes,
31743205
nextElt->getArgumentInterfaceType(),
31753206
/*excluding*/theEnum->getParentModule());
@@ -3214,11 +3245,28 @@ void Serializer::writeDecl(const Decl *D) {
32143245
auto conformances = theClass->getLocalConformances(
32153246
ConformanceLookupKind::NonInherited, nullptr);
32163247

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

32233271
uint8_t rawAccessLevel =
32243272
getRawStableAccessLevel(theClass->getFormalAccess());
@@ -3240,7 +3288,8 @@ void Serializer::writeDecl(const Decl *D) {
32403288
addTypeRef(theClass->getSuperclass()),
32413289
rawAccessLevel,
32423290
conformances.size(),
3243-
inheritedTypes);
3291+
theClass->getInherited().size(),
3292+
inheritedAndDependencyTypes);
32443293

32453294
writeGenericParams(theClass->getGenericParams());
32463295
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)