-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[Serialization] Drop a class if the superclass can't be found #24819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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<const ExtensionDecl *, 8> seenExtensionDCs; | ||
|
|
||
| const DeclContext *dc = decl; | ||
| do { | ||
| if (dc == problemContext) | ||
| return true; | ||
|
|
||
| if (auto *extension = dyn_cast<ExtensionDecl>(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<Type, 4> &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<Type, 4> &seen, | |
| static void | ||
| collectDependenciesFromRequirement(llvm::SmallSetVector<Type, 4> &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<TypeID, 4> inheritedTypes; | ||
| SmallVector<TypeID, 4> inheritedAndDependencyTypes; | ||
| for (auto inherited : theStruct->getInherited()) { | ||
| assert(!inherited.getType()->hasArchetype()); | ||
| inheritedTypes.push_back(addTypeRef(inherited.getType())); | ||
| inheritedAndDependencyTypes.push_back(addTypeRef(inherited.getType())); | ||
| } | ||
|
|
||
| llvm::SmallSetVector<Type, 4> 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<TypeID, 4> inheritedTypes; | ||
| SmallVector<TypeID, 4> inheritedAndDependencyTypes; | ||
| for (auto inherited : theClass->getInherited()) { | ||
| assert(!inherited.getType()->hasArchetype()); | ||
| inheritedTypes.push_back(addTypeRef(inherited.getType())); | ||
| inheritedAndDependencyTypes.push_back(addTypeRef(inherited.getType())); | ||
| } | ||
|
|
||
| llvm::SmallSetVector<Type, 4> 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); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| @interface Root | ||
| - (instancetype)init; | ||
| @end | ||
|
|
||
| #if !BAD | ||
| @interface DisappearingSuperclass : Root | ||
| @end | ||
| #else | ||
| #endif |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<T> where T : DisappearingSuperclass { | ||
| // CHECK-RECOVERY-NOT: class B1_ConstrainedGeneric | ||
| public class B1_ConstrainedGeneric<T> where T: DisappearingSuperclass {} | ||
|
|
||
| // CHECK-LABEL: struct B2_ConstrainedGeneric<T> where T : DisappearingSuperclass { | ||
| // CHECK-RECOVERY-NOT: struct B2_ConstrainedGeneric | ||
| public struct B2_ConstrainedGeneric<T> where T: DisappearingSuperclass {} | ||
|
|
||
| // CHECK-ALWAYS-LABEL: class C1_GenericBase<T> { | ||
| public class C1_GenericBase<T> {} | ||
|
|
||
| // CHECK-ALWAYS-LABEL: class C2_CRTPish : C1_GenericBase<C2_CRTPish> { | ||
| public class C2_CRTPish: C1_GenericBase<C2_CRTPish> {} | ||
| // CHECK-ALWAYS-LABEL: class C3_NestingCRTPish : C1_GenericBase<C3_NestingCRTPish.Nested> { | ||
| public class C3_NestingCRTPish: C1_GenericBase<C3_NestingCRTPish.Nested> { | ||
| public struct Nested {} | ||
| } | ||
| // CHECK-ALWAYS-LABEL: class C4_ExtensionNestingCRTPish : C1_GenericBase<C4_ExtensionNestingCRTPish.Nested> { | ||
| public class C4_ExtensionNestingCRTPish: C1_GenericBase<C4_ExtensionNestingCRTPish.Nested> {} | ||
| extension C4_ExtensionNestingCRTPish { | ||
| public struct Nested {} | ||
| } | ||
|
|
||
| #endif // TEST |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| @interface Root | ||
| - (instancetype)init; | ||
| @end | ||
|
|
||
| #if !BAD | ||
| @interface DisappearingSuperclass : Root | ||
| @end | ||
| #else | ||
| #endif |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| module rdar40899824Helper { header "rdar40899824Helper.h" } | ||
| module SuperclassObjC { header "SuperclassObjC.h" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it right for inheritance clause entries to add dependencies? The change I'm working on just drops ones that don't deserialize. I think only the superclass and raw type of an enum should introduce dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The inheritance clause doesn't add dependencies; it's just the silly LLVM bitstream container format that forces us to put both dependencies and inherited types in the same array. That's what the
sliceis for.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, oops.