From aeb0fedad10f28f19024fb2d6b6c4e69efb68fda Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Tue, 9 May 2017 18:00:21 -0700 Subject: [PATCH] Handle missing members in protocols as well. This means both not crashing when we deserialize the protocol but also emitting correct offsets for dynamic dispatch through the protocol's witness table. Also fix a bug with vtable and witness table slots for materializeForSet accessors for properties that can't be imported. Because materializeForSet doesn't have the type of the property in its signature, it was taking a different failure path from everything else, and that failure path didn't properly set the name or flags for the missing member. Finishes rdar://problem/31878396 --- include/swift/SIL/SILWitnessVisitor.h | 4 + lib/IRGen/GenProto.cpp | 11 ++ lib/SILGen/SILGenType.cpp | 8 ++ lib/Serialization/Deserialization.cpp | 27 +++- .../Recovery/typedefs-in-protocols.swift | 132 ++++++++++++++++++ test/Serialization/Recovery/typedefs.swift | 37 ++--- 6 files changed, 200 insertions(+), 19 deletions(-) create mode 100644 test/Serialization/Recovery/typedefs-in-protocols.swift diff --git a/include/swift/SIL/SILWitnessVisitor.h b/include/swift/SIL/SILWitnessVisitor.h index 5d3e9a97b95..0c86079e898 100644 --- a/include/swift/SIL/SILWitnessVisitor.h +++ b/include/swift/SIL/SILWitnessVisitor.h @@ -143,6 +143,10 @@ public: asDerived().addMethod(func); } + void visitMissingMemberDecl(MissingMemberDecl *placeholder) { + asDerived().addPlaceholder(placeholder); + } + void visitAssociatedTypeDecl(AssociatedTypeDecl *td) { // We already visited these in the first pass. } diff --git a/lib/IRGen/GenProto.cpp b/lib/IRGen/GenProto.cpp index b6aae1813bc..274118688be 100644 --- a/lib/IRGen/GenProto.cpp +++ b/lib/IRGen/GenProto.cpp @@ -662,6 +662,13 @@ namespace { Entries.push_back(WitnessTableEntry::forFunction(ctor)); } + void addPlaceholder(MissingMemberDecl *placeholder) { + for (auto i : range(placeholder->getNumberOfVTableEntries())) { + (void)i; + Entries.push_back(WitnessTableEntry()); + } + } + void addAssociatedType(AssociatedTypeDecl *ty) { Entries.push_back(WitnessTableEntry::forAssociatedType(ty)); } @@ -1117,6 +1124,10 @@ public: return addMethodFromSILWitnessTable(requirement); } + void addPlaceholder(MissingMemberDecl *placeholder) { + llvm_unreachable("cannot emit a witness table with placeholders in it"); + } + void addAssociatedType(AssociatedTypeDecl *requirement) { #ifndef NDEBUG auto &entry = SILEntries.front(); diff --git a/lib/SILGen/SILGenType.cpp b/lib/SILGen/SILGenType.cpp index 58f3f2e33f7..53c96cfecb1 100644 --- a/lib/SILGen/SILGenType.cpp +++ b/lib/SILGen/SILGenType.cpp @@ -393,6 +393,10 @@ public: super::addConstructor(cd, witness); } + void addPlaceholder(MissingMemberDecl *placeholder) { + llvm_unreachable("generating a witness table with placeholders in it"); + } + void addMethod(SILDeclRef requirementRef, SILDeclRef witnessRef, IsFreeFunctionWitness_t isFree, @@ -727,6 +731,10 @@ public: super::addConstructor(cd, witness); } + void addPlaceholder(MissingMemberDecl *placeholder) { + llvm_unreachable("generating a witness table with placeholders in it"); + } + void addMethod(SILDeclRef requirementRef, SILDeclRef witnessRef, IsFreeFunctionWitness_t isFree, diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index 1615661c9ce..23cec79846a 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -2866,8 +2866,11 @@ ModuleFile::getDeclChecked(DeclID DID, Optional ForcedContext) { // If we are an accessor on a var or subscript, make sure it is deserialized // first. auto accessor = getDeclChecked(accessorStorageDeclID); - if (!accessor) - return accessor.takeError(); + if (!accessor) { + // FIXME: "TypeError" isn't exactly correct for this. + return llvm::make_error( + name, takeErrorInfo(accessor.takeError()), errorFlags); + } // Read generic params before reading the type, because the type may // reference generic parameters, and we want them to have a dummy @@ -4468,6 +4471,26 @@ void ModuleFile::loadAllMembers(Decl *container, uint64_t contextData) { // subscripts, and methods that don't need vtable entries. }; llvm::handleAllErrors(next.takeError(), handleMissingClassMember); + } else if (auto *containingProto = dyn_cast(container)) { + auto handleMissingProtocolMember = + [&](const DeclDeserializationError &error) { + assert(!error.needsAllocatingVTableEntry()); + if (error.needsVTableEntry()) + containingProto->setHasMissingRequirements(true); + + if (error.getName().getBaseName() == getContext().Id_init) { + members.push_back(MissingMemberDecl::forInitializer( + getContext(), containingProto, error.getName(), + error.needsVTableEntry(), error.needsAllocatingVTableEntry())); + } else if (error.needsVTableEntry()) { + members.push_back(MissingMemberDecl::forMethod( + getContext(), containingProto, error.getName(), + error.needsVTableEntry())); + } + // FIXME: Handle other kinds of missing members: properties, + // subscripts, and methods that don't need vtable entries. + }; + llvm::handleAllErrors(next.takeError(), handleMissingProtocolMember); } else { llvm::consumeError(next.takeError()); } diff --git a/test/Serialization/Recovery/typedefs-in-protocols.swift b/test/Serialization/Recovery/typedefs-in-protocols.swift new file mode 100644 index 00000000000..cd4eb774f8a --- /dev/null +++ b/test/Serialization/Recovery/typedefs-in-protocols.swift @@ -0,0 +1,132 @@ +// RUN: rm -rf %t && mkdir -p %t +// RUN: %target-swift-frontend -emit-sil -o - -emit-module-path %t/Lib.swiftmodule -module-name Lib -I %S/Inputs/custom-modules -disable-objc-attr-requires-foundation-module %s | %FileCheck -check-prefix CHECK-WITNESS-TABLE %s + +// RUN: %target-swift-ide-test -source-filename=x -print-module -module-to-print Lib -I %t -I %S/Inputs/custom-modules | %FileCheck %s + +// RUN: %target-swift-ide-test -source-filename=x -print-module -module-to-print Lib -I %t -I %S/Inputs/custom-modules -Xcc -DBAD -enable-experimental-deserialization-recovery | %FileCheck -check-prefix CHECK-RECOVERY %s + +// RUN: %target-swift-frontend -typecheck -I %t -I %S/Inputs/custom-modules -Xcc -DBAD -DTEST -enable-experimental-deserialization-recovery -DVERIFY %s -verify + +// RUN: %target-swift-frontend -emit-ir -I %t -I %S/Inputs/custom-modules -DTEST %s | %FileCheck -check-prefix CHECK-IR %s +// RUN: %target-swift-frontend -emit-ir -I %t -I %S/Inputs/custom-modules -Xcc -DBAD -DTEST -enable-experimental-deserialization-recovery %s | %FileCheck -check-prefix CHECK-IR %s + +#if TEST + +import Typedefs +import Lib + +// CHECK-IR-LABEL: define{{.*}} void @_T04main19testWitnessDispatch +public func testWitnessDispatch(user: Proto) { + // The important thing in this CHECK line is the "i64 11", which is the offset + // for the witness table slot for 'lastMethod()'. If the layout here + // changes, please check that offset 11 is still correct. + // CHECK-IR-NOT: ret + // CHECK-IR: [[SLOT:%.+]] = getelementptr inbounds i8*, i8** {{%.+}}, i32 11 + // CHECK-IR-NOT: ret + // CHECK-IR: [[RAW_METHOD:%.+]] = load i8*, i8** [[SLOT]] + // CHECK-IR-NOT: ret + // CHECK-IR: [[METHOD:%.+]] = bitcast i8* [[RAW_METHOD]] to void (%swift.opaque*, %swift.type*, i8**)* + // CHECK-IR-NOT: ret + // CHECK-IR: call swiftcc void [[METHOD]]( + _ = user.lastMethod() +} // CHECK-IR: ret void + +// CHECK-IR-LABEL: define{{.*}} void @_T04main19testGenericDispatch +public func testGenericDispatch(user: T) { + // The important thing in this CHECK line is the "i64 11", which is the offset + // for the witness table slot for 'lastMethod()'. If the layout here + // changes, please check that offset 11 is still correct. + // CHECK-IR-NOT: ret + // CHECK-IR: [[SLOT:%.+]] = getelementptr inbounds i8*, i8** %T.Proto, i32 11 + // CHECK-IR-NOT: ret + // CHECK-IR: [[RAW_METHOD:%.+]] = load i8*, i8** [[SLOT]] + // CHECK-IR-NOT: ret + // CHECK-IR: [[METHOD:%.+]] = bitcast i8* [[RAW_METHOD]] to void (%swift.opaque*, %swift.type*, i8**)* + // CHECK-IR-NOT: ret + // CHECK-IR: call swiftcc void [[METHOD]]( + _ = user.lastMethod() +} // CHECK-IR: ret void + +#if VERIFY + +public class TestImpl : Proto {} // expected-error {{type 'TestImpl' cannot conform to protocol 'Proto' because it has requirements that cannot be satisfied}} + +#endif // VERIFY + +#else // TEST + +import Typedefs + +// CHECK-LABEL: protocol Proto { +// CHECK-RECOVERY-LABEL: protocol Proto { +public protocol Proto { + // CHECK: var unwrappedProp: UnwrappedInt? { get set } + // CHECK-RECOVERY: var unwrappedProp: Int32? + var unwrappedProp: UnwrappedInt? { get set } + // CHECK: var wrappedProp: WrappedInt? { get set } + // CHECK-RECOVERY: /* placeholder for _ */ + // CHECK-RECOVERY: /* placeholder for _ */ + // CHECK-RECOVERY: /* placeholder for _ */ + var wrappedProp: WrappedInt? { get set } + + // CHECK: func returnsUnwrappedMethod() -> UnwrappedInt + // CHECK-RECOVERY: func returnsUnwrappedMethod() -> Int32 + func returnsUnwrappedMethod() -> UnwrappedInt + // CHECK: func returnsWrappedMethod() -> WrappedInt + // CHECK-RECOVERY: /* placeholder for returnsWrappedMethod() */ + func returnsWrappedMethod() -> WrappedInt + + // CHECK: subscript(_: WrappedInt) -> () { get } + // CHECK-RECOVERY: /* placeholder for _ */ + subscript(_: WrappedInt) -> () { get } + + // CHECK: init() + // CHECK-RECOVERY: init() + init() + + // CHECK: init(wrapped: WrappedInt) + // CHECK-RECOVERY: /* placeholder for init(wrapped:) */ + init(wrapped: WrappedInt) + + func lastMethod() +} +// CHECK: {{^}$}} +// CHECK-RECOVERY: {{^}$}} + +// CHECK-LABEL: struct ProtoLibImpl : Proto { +// CHECK-RECOVERY-LABEL: struct ProtoLibImpl : Proto { +public struct ProtoLibImpl : Proto { + public var unwrappedProp: UnwrappedInt? + public var wrappedProp: WrappedInt? + + public func returnsUnwrappedMethod() -> UnwrappedInt { fatalError() } + public func returnsWrappedMethod() -> WrappedInt { fatalError() } + + public subscript(_: WrappedInt) -> () { return () } + + public init() {} + public init(wrapped: WrappedInt) {} + + public func lastMethod() {} +} +// CHECK: {{^}$}} +// CHECK-RECOVERY: {{^}$}} + +// This is mostly to check when changes are necessary for the CHECK-IR lines +// above. +// CHECK-WITNESS-TABLE-LABEL: sil_witness_table{{.*}} ProtoLibImpl: Proto module Lib { +// 0 CHECK-WITNESS-TABLE-NEXT: #Proto.unwrappedProp!getter.1: +// 1 CHECK-WITNESS-TABLE-NEXT: #Proto.unwrappedProp!setter.1: +// 2 CHECK-WITNESS-TABLE-NEXT: #Proto.unwrappedProp!materializeForSet.1: +// 3 CHECK-WITNESS-TABLE-NEXT: #Proto.wrappedProp!getter.1: +// 4 CHECK-WITNESS-TABLE-NEXT: #Proto.wrappedProp!setter.1: +// 5 CHECK-WITNESS-TABLE-NEXT: #Proto.wrappedProp!materializeForSet.1: +// 6 CHECK-WITNESS-TABLE-NEXT: #Proto.returnsUnwrappedMethod!1: +// 7 CHECK-WITNESS-TABLE-NEXT: #Proto.returnsWrappedMethod!1: +// 8 CHECK-WITNESS-TABLE-NEXT: #Proto.subscript!getter.1: +// 9 CHECK-WITNESS-TABLE-NEXT: #Proto.init!allocator.1: +// 10 CHECK-WITNESS-TABLE-NEXT: #Proto.init!allocator.1: +// 11 CHECK-WITNESS-TABLE-NEXT: #Proto.lastMethod!1: +// CHECK-WITNESS-TABLE: } + +#endif // TEST diff --git a/test/Serialization/Recovery/typedefs.swift b/test/Serialization/Recovery/typedefs.swift index 91845802881..e33209aa206 100644 --- a/test/Serialization/Recovery/typedefs.swift +++ b/test/Serialization/Recovery/typedefs.swift @@ -9,6 +9,8 @@ // RUN: %target-swift-frontend -typecheck -I %t -I %S/Inputs/custom-modules -Xcc -DBAD -DTEST -enable-experimental-deserialization-recovery -DVERIFY %s -verify // RUN: %target-swift-frontend -emit-silgen -I %t -I %S/Inputs/custom-modules -Xcc -DBAD -DTEST -enable-experimental-deserialization-recovery %s | %FileCheck -check-prefix CHECK-SIL %s + +// RUN: %target-swift-frontend -emit-ir -I %t -I %S/Inputs/custom-modules -DTEST %s | %FileCheck -check-prefix CHECK-IR %s // RUN: %target-swift-frontend -emit-ir -I %t -I %S/Inputs/custom-modules -Xcc -DBAD -DTEST -enable-experimental-deserialization-recovery %s | %FileCheck -check-prefix CHECK-IR %s #if TEST @@ -27,11 +29,11 @@ func testSymbols() { // CHECK-IR-LABEL: define{{.*}} void @_T08typedefs18testVTableBuildingy3Lib4UserC4user_tF public func testVTableBuilding(user: User) { - // The important thing in this CHECK line is the "i64 23", which is the offset + // The important thing in this CHECK line is the "i64 24", which is the offset // for the vtable slot for 'lastMethod()'. If the layout here - // changes, please check that offset 23 is still correct. + // changes, please check that offset 24 is still correct. // CHECK-IR-NOT: ret - // CHECK-IR: getelementptr inbounds void (%T3Lib4UserC*)*, void (%T3Lib4UserC*)** %{{[0-9]+}}, i64 23 + // CHECK-IR: getelementptr inbounds void (%T3Lib4UserC*)*, void (%T3Lib4UserC*)** %{{[0-9]+}}, i64 24 _ = user.lastMethod() } // CHECK-IR: ret void @@ -77,6 +79,7 @@ open class User { // CHECK: var wrappedProp: WrappedInt? // CHECK-RECOVERY: /* placeholder for _ */ // CHECK-RECOVERY: /* placeholder for _ */ + // CHECK-RECOVERY: /* placeholder for _ */ public var wrappedProp: WrappedInt? // CHECK: func returnsUnwrappedMethod() -> UnwrappedInt @@ -114,22 +117,22 @@ open class User { // This is mostly to check when changes are necessary for the CHECK-IR lines // above. // CHECK-VTABLE-LABEL: sil_vtable User { -// (8 words of normal class metadata) -// 9 CHECK-VTABLE-NEXT: #User.unwrappedProp!getter.1: -// 10 CHECK-VTABLE-NEXT: #User.unwrappedProp!setter.1: -// 11 CHECK-VTABLE-NEXT: #User.unwrappedProp!materializeForSet.1: -// 12 CHECK-VTABLE-NEXT: #User.wrappedProp!getter.1: -// 13 CHECK-VTABLE-NEXT: #User.wrappedProp!setter.1: -// 14 CHECK-VTABLE-NEXT: #User.wrappedProp!materializeForSet.1: -// 15 CHECK-VTABLE-NEXT: #User.returnsUnwrappedMethod!1: -// 16 CHECK-VTABLE-NEXT: #User.returnsWrappedMethod!1: -// 17 CHECK-VTABLE-NEXT: #User.subscript!getter.1: -// 18 CHECK-VTABLE-NEXT: #User.init!initializer.1: +// (10 words of normal class metadata) +// 10 CHECK-VTABLE-NEXT: #User.unwrappedProp!getter.1: +// 11 CHECK-VTABLE-NEXT: #User.unwrappedProp!setter.1: +// 12 CHECK-VTABLE-NEXT: #User.unwrappedProp!materializeForSet.1: +// 13 CHECK-VTABLE-NEXT: #User.wrappedProp!getter.1: +// 14 CHECK-VTABLE-NEXT: #User.wrappedProp!setter.1: +// 15 CHECK-VTABLE-NEXT: #User.wrappedProp!materializeForSet.1: +// 16 CHECK-VTABLE-NEXT: #User.returnsUnwrappedMethod!1: +// 17 CHECK-VTABLE-NEXT: #User.returnsWrappedMethod!1: +// 18 CHECK-VTABLE-NEXT: #User.subscript!getter.1: // 19 CHECK-VTABLE-NEXT: #User.init!initializer.1: // 20 CHECK-VTABLE-NEXT: #User.init!initializer.1: -// 21 CHECK-VTABLE-NEXT: #User.init!allocator.1: -// 22 CHECK-VTABLE-NEXT: #User.init!initializer.1: -// 23 CHECK-VTABLE-NEXT: #User.lastMethod!1: +// 21 CHECK-VTABLE-NEXT: #User.init!initializer.1: +// 22 CHECK-VTABLE-NEXT: #User.init!allocator.1: +// 23 CHECK-VTABLE-NEXT: #User.init!initializer.1: +// 24 CHECK-VTABLE-NEXT: #User.lastMethod!1: // CHECK-VTABLE: }