From af8d13e9d3b1a5f810368588131988912f475880 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Sun, 23 Oct 2016 23:53:38 -0700 Subject: [PATCH 1/5] stdlib: Make UnicodeDecodingResult fixed-layout --- stdlib/public/core/Unicode.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/stdlib/public/core/Unicode.swift b/stdlib/public/core/Unicode.swift index c2f3a2948930f..86bad386c1cb8 100644 --- a/stdlib/public/core/Unicode.swift +++ b/stdlib/public/core/Unicode.swift @@ -22,6 +22,7 @@ import SwiftShims /// of a decoding error. /// /// - SeeAlso: `UnicodeCodec.decode(next:)` +@_fixed_layout public enum UnicodeDecodingResult : Equatable { /// A decoded Unicode scalar value. case scalarValue(UnicodeScalar) From 0bc3dec95461a7150794df63bb79ab3f23ec0040 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 24 Oct 2016 00:16:43 -0700 Subject: [PATCH 2/5] Sema: New TypeBase::adjustSuperclassMemberDeclType() method This generalizes some code in Sema to fix the problem where generic method overrides don't work if the base class is more or less generic than the derived class. The problem here was that we were checking types for equality when matching overrides, which failed if generic parameters had different depths. Now, map the generic parameters of the base class member to the generic signature of the derived member, so that the equality check can succeed. Since SIL type lowering needs to perform a similar check, move this from Sema to a method on TypeBase to complement the existing getTypeOfMember(). Note that getTypeOfMember() still does a superclass walk, but ideally this will go away soon. --- include/swift/AST/Types.h | 8 +++++++ lib/AST/Type.cpp | 44 +++++++++++++++++++++++++++++++++++++- lib/Sema/TypeCheckDecl.cpp | 40 ++++++---------------------------- 3 files changed, 58 insertions(+), 34 deletions(-) diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h index 4c9a7522f4fef..86fb0cfdae173 100644 --- a/include/swift/AST/Types.h +++ b/include/swift/AST/Types.h @@ -866,6 +866,14 @@ class alignas(1 << TypeAlignInBits) TypeBase { Type getTypeOfMember(ModuleDecl *module, Type memberType, const DeclContext *memberDC); + /// Get the type of a superclass member as seen from the subclass, + /// substituting generic parameters, dynamic Self return, and the + /// 'self' argument type as appropriate. + Type adjustSuperclassMemberDeclType(const ValueDecl *decl, + const ValueDecl *parentDecl, + Type memberType, + LazyResolver *resolver); + /// Return T if this type is Optional; otherwise, return the null type. Type getOptionalObjectType(); diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index 18b9695e09c1e..235c48f6c9d05 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -3053,7 +3053,7 @@ Type TypeBase::getTypeOfMember(Module *module, Type memberType, return memberType; // Compute the set of member substitutions to apply. - TypeSubstitutionMap substitutions = getMemberSubstitutions(memberDC); + auto substitutions = getMemberSubstitutions(memberDC); if (substitutions.empty()) return memberType; @@ -3061,6 +3061,48 @@ Type TypeBase::getTypeOfMember(Module *module, Type memberType, return memberType.subst(module, substitutions, None); } +Type TypeBase::adjustSuperclassMemberDeclType(const ValueDecl *decl, + const ValueDecl *parentDecl, + Type memberType, + LazyResolver *resolver) { + auto *DC = parentDecl->getDeclContext(); + auto superclass = getSuperclassForDecl( + DC->getAsClassOrClassExtensionContext(), resolver); + auto subs = superclass->getMemberSubstitutions(DC); + + if (auto *parentFunc = dyn_cast(parentDecl)) { + if (auto *func = dyn_cast(decl)) { + auto *genericParams = func->getGenericParams(); + auto *parentParams = parentFunc->getGenericParams(); + if (genericParams && parentParams && + genericParams->size() == parentParams->size()) { + for (unsigned i = 0, e = genericParams->size(); i < e; i++) { + auto paramTy = parentParams->getParams()[i]->getDeclaredInterfaceType() + ->getCanonicalType().getPointer(); + subs[paramTy] = genericParams->getParams()[i] + ->getDeclaredInterfaceType(); + } + } + } + } + + auto type = memberType.subst(parentDecl->getModuleContext(), subs); + + if (isa(parentDecl)) { + type = type->replaceSelfParameterType(this); + if (auto func = dyn_cast(parentDecl)) { + if (func->hasDynamicSelf()) { + type = type->replaceCovariantResultType(this, + func->getNumParameterLists()); + } + } else if (isa(parentDecl)) { + type = type->replaceCovariantResultType(this, /*uncurryLevel=*/2); + } + } + + return type; +} + Identifier DependentMemberType::getName() const { if (NameOrAssocType.is()) return NameOrAssocType.get(); diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index a6dd871f1e584..bc665bb138a32 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -5014,32 +5014,6 @@ class DeclChecker : public DeclVisitor { void visitModuleDecl(ModuleDecl *) { } - /// Adjust the type of the given declaration to appear as if it were - /// in the given subclass of its actual declared class. - static Type adjustSuperclassMemberDeclType(TypeChecker &TC, - const ValueDecl *decl, - Type subclass) { - ClassDecl *superclassDecl = - decl->getDeclContext()->getDeclaredTypeInContext() - ->getClassOrBoundGenericClass(); - auto superclass = subclass; - while (superclass->getClassOrBoundGenericClass() != superclassDecl) - superclass = TC.getSuperClassOf(superclass); - auto type = TC.substMemberTypeWithBase(decl->getModuleContext(), decl, - superclass, - /*isTypeReference=*/false); - if (auto func = dyn_cast(decl)) { - if (func->hasDynamicSelf()) { - type = type->replaceCovariantResultType(subclass, - func->getNumParameterLists()); - } - } else if (isa(decl)) { - type = type->replaceCovariantResultType(subclass, /*uncurryLevel=*/2); - } - - return type; - } - /// Perform basic checking to determine whether a declaration can override a /// declaration in a superclass. static bool areOverrideCompatibleSimple(ValueDecl *decl, @@ -5093,15 +5067,15 @@ class DeclChecker : public DeclVisitor { static bool diagnoseMismatchedOptionals(TypeChecker &TC, - const Decl *member, + const ValueDecl *member, const ParameterList *params, TypeLoc resultTL, const ValueDecl *parentMember, Type owningTy, bool treatIUOResultAsError) { bool emittedError = false; - Type plainParentTy = adjustSuperclassMemberDeclType(TC, parentMember, - owningTy); + Type plainParentTy = owningTy->adjustSuperclassMemberDeclType( + member, parentMember, parentMember->getInterfaceType(), &TC); const auto *parentTy = plainParentTy->castTo(); if (isa(parentMember)) parentTy = parentTy->getResult()->castTo(); @@ -5538,8 +5512,8 @@ class DeclChecker : public DeclVisitor { // Check whether the types are identical. // FIXME: It's wrong to use the uncurried types here for methods. - auto parentDeclTy = adjustSuperclassMemberDeclType(TC, parentDecl, - owningTy); + auto parentDeclTy = owningTy->adjustSuperclassMemberDeclType( + decl, parentDecl, parentDecl->getInterfaceType(), &TC); parentDeclTy = parentDeclTy->getUnlabeledType(TC.Context); if (method) { parentDeclTy = parentDeclTy->castTo()->getResult(); @@ -5807,8 +5781,8 @@ class DeclChecker : public DeclVisitor { } } else if (auto property = dyn_cast_or_null(abstractStorage)) { auto propertyTy = property->getInterfaceType(); - auto parentPropertyTy = adjustSuperclassMemberDeclType(TC, matchDecl, - superclass); + auto parentPropertyTy = superclass->adjustSuperclassMemberDeclType( + decl, matchDecl, matchDecl->getInterfaceType(), &TC); if (!propertyTy->canOverride(parentPropertyTy, OverrideMatchMode::Strict, From dec05e14e1a237a86664e91449e37d00da5d9d57 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 24 Oct 2016 00:19:20 -0700 Subject: [PATCH 3/5] Sema: Simplify substMemberTypeWithBase() The isTypeReference parameter is now always true, so change the function to just take a TypeDecl. --- lib/Sema/TypeCheckGeneric.cpp | 2 +- lib/Sema/TypeCheckNameLookup.cpp | 3 +-- lib/Sema/TypeCheckType.cpp | 34 ++++++++++++++------------------ lib/Sema/TypeChecker.h | 9 +++------ 4 files changed, 20 insertions(+), 28 deletions(-) diff --git a/lib/Sema/TypeCheckGeneric.cpp b/lib/Sema/TypeCheckGeneric.cpp index c85abaa7f7804..f12b9ed22451f 100644 --- a/lib/Sema/TypeCheckGeneric.cpp +++ b/lib/Sema/TypeCheckGeneric.cpp @@ -206,7 +206,7 @@ Type CompleteGenericTypeResolver::resolveDependentMemberType( // concrete type, or resolve its components down to another dependent member. if (auto alias = nestedPA->getTypeAliasDecl()) { return TC.substMemberTypeWithBase(DC->getParentModule(), alias, - baseTy, true); + baseTy); } Identifier name = ref->getIdentifier(); diff --git a/lib/Sema/TypeCheckNameLookup.cpp b/lib/Sema/TypeCheckNameLookup.cpp index 99e58a4c8859c..bf6560e0f6ff1 100644 --- a/lib/Sema/TypeCheckNameLookup.cpp +++ b/lib/Sema/TypeCheckNameLookup.cpp @@ -369,8 +369,7 @@ LookupTypeResult TypeChecker::lookupMemberType(DeclContext *dc, // Substitute the base into the member's type. auto memberType = substMemberTypeWithBase(dc->getParentModule(), - typeDecl, type, - /*isTypeReference=*/true); + typeDecl, type); // FIXME: It is not clear why this substitution can fail, but the // standard library won't build without this check. diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index 46bf9c7bb1258..e0c75d9780fb4 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -337,7 +337,7 @@ Type TypeChecker::resolveTypeInContext( } return substMemberTypeWithBase(parentDC->getParentModule(), typeDecl, - fromType, /*isTypeReference=*/true); + fromType); } } @@ -378,7 +378,7 @@ Type TypeChecker::resolveTypeInContext( } return substMemberTypeWithBase(parentDC->getParentModule(), typeDecl, - fromType, /*isTypeReference=*/true); + fromType); } if (auto superclassTy = getSuperClassOf(fromType)) @@ -1142,8 +1142,7 @@ static Type resolveNestedIdentTypeComponent( // Otherwise, simply substitute the parent type into the member. memberType = TC.substMemberTypeWithBase(DC->getParentModule(), typeDecl, - parentTy, - /*isTypeReference=*/true); + parentTy); // Propagate failure. if (!memberType || memberType->hasError()) return memberType; @@ -2797,21 +2796,18 @@ Type TypeResolver::buildProtocolType( } Type TypeChecker::substMemberTypeWithBase(Module *module, - const ValueDecl *member, - Type baseTy, bool isTypeReference) { - Type memberType = isTypeReference - ? cast(member)->getDeclaredInterfaceType() - : member->getInterfaceType(); - if (isTypeReference) { - // The declared interface type for a generic type will have the type - // arguments; strip them off. - if (auto nominalTypeDecl = dyn_cast(member)) { - if (auto boundGenericTy = memberType->getAs()) { - memberType = UnboundGenericType::get( - const_cast(nominalTypeDecl), - boundGenericTy->getParent(), - Context); - } + const TypeDecl *member, + Type baseTy) { + Type memberType = member->getDeclaredInterfaceType(); + + // The declared interface type for a generic type will have the type + // arguments; strip them off. + if (auto nominalTypeDecl = dyn_cast(member)) { + if (auto boundGenericTy = memberType->getAs()) { + memberType = UnboundGenericType::get( + const_cast(nominalTypeDecl), + boundGenericTy->getParent(), + Context); } } diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index b207b2790e813..c658488944ea7 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -855,17 +855,14 @@ class TypeChecker final : public LazyResolver { bool isGenericSignature, GenericTypeResolver *resolver); - /// \brief Substitute the given base type into the type of the given member, - /// producing the effective type that the member will have. + /// \brief Substitute the given base type into the type of the given nested type, + /// producing the effective type that the nested type will have. /// /// \param module The module in which the substitution will be performed. /// \param member The member whose type projection is being computed. /// \param baseTy The base type that will be substituted for the 'Self' of the /// member. - /// \param isTypeReference Whether this is a reference to a type declaration - /// within a type context. - Type substMemberTypeWithBase(Module *module, const ValueDecl *member, - Type baseTy, bool isTypeReference); + Type substMemberTypeWithBase(Module *module, const TypeDecl *member, Type baseTy); /// \brief Retrieve the superclass type of the given type, or a null type if /// the type has no supertype. From 4b2bb8ff97c3507390bdd826636e5226c8ca55e1 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 24 Oct 2016 01:36:00 -0700 Subject: [PATCH 4/5] SIL: Simpler and more correct TypeLowering::getConstantOverrideInfo() We can use the new adjustSuperclassMemberDeclType() method here to eliminate some code duplication and fix some corner cases with generic method overrides. Fixes , , . --- lib/SIL/SILFunctionType.cpp | 29 +++++------------- test/SILGen/vtable_thunks_reabstraction.swift | 30 +++++++++++++++++++ test/attr/attr_override.swift | 21 +++++++++++++ 3 files changed, 59 insertions(+), 21 deletions(-) diff --git a/lib/SIL/SILFunctionType.cpp b/lib/SIL/SILFunctionType.cpp index f79386c549f8e..92f685f66bd1c 100644 --- a/lib/SIL/SILFunctionType.cpp +++ b/lib/SIL/SILFunctionType.cpp @@ -2041,10 +2041,14 @@ SILConstantInfo TypeConverter::getConstantOverrideInfo(SILDeclRef derived, auto selfInterfaceTy = derivedInterfaceTy.getInput()->getRValueInstanceType(); auto overrideInterfaceTy = - selfInterfaceTy->getTypeOfMember(M.getSwiftModule(), baseInterfaceTy, - base.getDecl()->getDeclContext()); - - // Copy generic signature from derived to the override type + selfInterfaceTy->adjustSuperclassMemberDeclType( + derived.getDecl(), base.getDecl(), baseInterfaceTy, + /*resolver=*/nullptr); + + // Copy generic signature from derived to the override type, to handle + // the case where the base member is not generic (because the base class + // is concrete) but the derived member is generic (because the derived + // class is generic). if (auto derivedInterfaceFnTy = derivedInterfaceTy->getAs()) { auto overrideInterfaceFnTy = overrideInterfaceTy->castTo(); overrideInterfaceTy = @@ -2054,23 +2058,6 @@ SILConstantInfo TypeConverter::getConstantOverrideInfo(SILDeclRef derived, overrideInterfaceFnTy->getExtInfo()); } - // Replace occurrences of 'Self' in the signature with the derived type. - // FIXME: these should all be modeled with a DynamicSelfType. - overrideInterfaceTy = - overrideInterfaceTy->replaceSelfParameterType(selfInterfaceTy); - - bool hasDynamicSelf = false; - if (auto funcDecl = dyn_cast(derived.getDecl())) - hasDynamicSelf = funcDecl->hasDynamicSelf(); - else if (isa(derived.getDecl())) - hasDynamicSelf = true; - - if (hasDynamicSelf) { - overrideInterfaceTy = - overrideInterfaceTy->replaceCovariantResultType(selfInterfaceTy, - base.uncurryLevel + 1); - } - // Lower the formal AST type. auto overrideLoweredInterfaceTy = getLoweredASTFunctionType( cast(overrideInterfaceTy->getCanonicalType()), diff --git a/test/SILGen/vtable_thunks_reabstraction.swift b/test/SILGen/vtable_thunks_reabstraction.swift index 39205448aaf32..e1edd14f7ceae 100644 --- a/test/SILGen/vtable_thunks_reabstraction.swift +++ b/test/SILGen/vtable_thunks_reabstraction.swift @@ -92,3 +92,33 @@ class ConcreteOptional: Opaque { override func variantOptionality(x: S??) -> S? { return x! } } */ + +// Make sure we remap the method's innermost generic parameters +// to the correct depth +class GenericBase { + func doStuff(t: T, u: U) {} + init(t: T, u: U) {} +} + +class ConcreteSub : GenericBase { + override func doStuff(t: Int, u: U) { + super.doStuff(t: t, u: u) + } + override init(t: Int, u: U) { + super.init(t: t, u: u) + } +} + +class ConcreteBase { + init(t: Int, u: U) {} + func doStuff(t: Int, u: U) {} +} + +class GenericSub : ConcreteBase { + override init(t: Int, u: U) { + super.init(t: t, u: u) + } + override func doStuff(t: Int, u: U) { + super.doStuff(t: t, u: u) + } +} diff --git a/test/attr/attr_override.swift b/test/attr/attr_override.swift index 9d4338154d881..1202653f69007 100644 --- a/test/attr/attr_override.swift +++ b/test/attr/attr_override.swift @@ -380,4 +380,25 @@ class MismatchOptional3 : MismatchOptionalBase { override func result() -> Optional { return nil } // expected-error {{cannot override instance method result type 'Int' with optional type 'Optional'}} {{none}} } +// Make sure we remap the method's innermost generic parameters +// to the correct depth +class GenericBase { + func doStuff(t: T, u: U) {} + init(t: T, u: U) {} +} + +class ConcreteSub : GenericBase { + override func doStuff(t: Int, u: U) {} + override init(t: Int, u: U) {} +} + +class ConcreteBase { + init(t: Int, u: U) {} + func doStuff(t: Int, u: U) {} +} + +class GenericSub : ConcreteBase { + override init(t: Int, u: U) {} + override func doStuff(t: Int, u: U) {} +} From 28764374a6f02d1f9a983bcc6dbf221b4d07395a Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 24 Oct 2016 01:51:28 -0700 Subject: [PATCH 5/5] stdlib: Remove workaround for rdar://15520519, resolving ABI FIXME#141 --- .../public/core/ContiguousArrayBuffer.swift | 39 +++++-------------- 1 file changed, 10 insertions(+), 29 deletions(-) diff --git a/stdlib/public/core/ContiguousArrayBuffer.swift b/stdlib/public/core/ContiguousArrayBuffer.swift index 98dd2e646af9f..316775d601d7c 100644 --- a/stdlib/public/core/ContiguousArrayBuffer.swift +++ b/stdlib/public/core/ContiguousArrayBuffer.swift @@ -59,13 +59,15 @@ internal var _emptyArrayStorage : _EmptyArrayStorage { Builtin.addressof(&_swiftEmptyArrayStorage)) } -// FIXME(ABI)#141 : This whole class is a workaround for -// Can't override generic method in generic -// subclass. If it weren't for that bug, we'd override -// _withVerbatimBridgedUnsafeBuffer directly in -// _ContiguousArrayStorage. -// rdar://problem/19341002 -class _ContiguousArrayStorage1 : _ContiguousArrayStorageBase { +// The class that implements the storage for a ContiguousArray +@_versioned +final class _ContiguousArrayStorage : _ContiguousArrayStorageBase { + + deinit { + _elementPointer.deinitialize(count: countAndCapacity.count) + _fixLifetime(self) + } + #if _runtime(_ObjC) /// If the `Element` is bridged verbatim, invoke `body` on an /// `UnsafeBufferPointer` to the elements and return the result. @@ -82,28 +84,7 @@ class _ContiguousArrayStorage1 : _ContiguousArrayStorageBase { /// If `Element` is bridged verbatim, invoke `body` on an /// `UnsafeBufferPointer` to the elements. - internal func _withVerbatimBridgedUnsafeBufferImpl( - _ body: (UnsafeBufferPointer) throws -> Void - ) rethrows { - _sanityCheckFailure( - "Must override _withVerbatimBridgedUnsafeBufferImpl in derived classes") - } -#endif -} - -// The class that implements the storage for a ContiguousArray -@_versioned -final class _ContiguousArrayStorage : _ContiguousArrayStorage1 { - - deinit { - _elementPointer.deinitialize(count: countAndCapacity.count) - _fixLifetime(self) - } - -#if _runtime(_ObjC) - /// If `Element` is bridged verbatim, invoke `body` on an - /// `UnsafeBufferPointer` to the elements. - internal final override func _withVerbatimBridgedUnsafeBufferImpl( + internal final func _withVerbatimBridgedUnsafeBufferImpl( _ body: (UnsafeBufferPointer) throws -> Void ) rethrows { if _isBridgedVerbatimToObjectiveC(Element.self) {