From 26349dc384eff49b193c4ed15455312bd56b3520 Mon Sep 17 00:00:00 2001 From: Joe Groff Date: Thu, 13 Sep 2018 18:20:15 -0700 Subject: [PATCH] Allow convenience initializers to reassign self. Now that convenience initializers are implemented exactly the same as value type delegating initializers, it's straightforward to allow them to assign self to an existing object reference instead of only allowing them to chain to other initializers. Because convenience inits can be inherited, the formal type of self is still the dynamic Self type, however. This was handled unsoundly in Swift 4 mode, and unfortunately we can't fix it without breaking source in Swift 4 mode, so enable this feature only in Swift 5 mode. Making `self` inout inside convenience inits also changes the code generation of loads from self, so we need to tweak DI's special handling of type(of: self) slightly so that it also recognizes this new pattern. --- lib/AST/ASTContext.cpp | 19 +++++---- lib/SIL/SILFunctionType.cpp | 7 +++- .../DIMemoryUseCollectorOwnership.cpp | 13 ++++++- .../Mandatory/DefiniteInitialization.cpp | 12 +++++- test/SILGen/objc_convenience_init.swift | 13 +++++++ ...nit_type_of_self_in_convenience_init.swift | 3 +- .../init/convenience_init_assignment_s4.swift | 26 +++++++++++++ .../init/convenience_init_assignment_s5.swift | 39 +++++++++++++++++++ 8 files changed, 119 insertions(+), 13 deletions(-) create mode 100644 test/SILGen/objc_convenience_init.swift create mode 100644 test/decl/init/convenience_init_assignment_s4.swift create mode 100644 test/decl/init/convenience_init_assignment_s5.swift diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index 10842063c5acf..549b99bbd450a 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -3098,6 +3098,7 @@ AnyFunctionType::Param swift::computeSelfParam(AbstractFunctionDecl *AFD, if (auto *FD = dyn_cast(AFD)) { isStatic = FD->isStatic(); + assert(!FD->isMutating() || !containerTy->hasReferenceSemantics()); isMutating = FD->isMutating(); // Methods returning 'Self' have a dynamic 'self'. @@ -3108,8 +3109,13 @@ AnyFunctionType::Param swift::computeSelfParam(AbstractFunctionDecl *AFD, } else if (auto *CD = dyn_cast(AFD)) { if (isInitializingCtor) { // initializing constructors of value types always have an implicitly - // inout self. - isMutating = true; + // inout self, as do convenience initializers in Swift 5 and later. + if (!containerTy->hasReferenceSemantics()) { + isMutating = true; + } else if (Ctx.LangOpts.isSwiftVersionAtLeast(5) + && !CD->isDesignatedInit()) { + isMutating = true; + } } else { // allocating constructors have metatype 'self'. isStatic = true; @@ -3123,8 +3129,9 @@ AnyFunctionType::Param swift::computeSelfParam(AbstractFunctionDecl *AFD, isDynamicSelf = true; } } else if (isa(AFD)) { - // destructors of value types always have an implicitly inout self. - isMutating = true; + // Destructors only appear on classes today. (If move-only types have + // destructors, they probably would want to consume self.) + assert(containerTy->hasReferenceSemantics()); } if (isDynamicSelf) @@ -3134,10 +3141,6 @@ AnyFunctionType::Param swift::computeSelfParam(AbstractFunctionDecl *AFD, if (isStatic) return AnyFunctionType::Param(MetatypeType::get(selfTy, Ctx)); - // Reference types have 'self' of type T. - if (containerTy->hasReferenceSemantics()) - return AnyFunctionType::Param(selfTy); - return AnyFunctionType::Param(selfTy, Identifier(), ParameterTypeFlags().withInOut(isMutating)); } diff --git a/lib/SIL/SILFunctionType.cpp b/lib/SIL/SILFunctionType.cpp index 08a04045565c2..9dc2dfb38f45a 100644 --- a/lib/SIL/SILFunctionType.cpp +++ b/lib/SIL/SILFunctionType.cpp @@ -697,7 +697,12 @@ class DestructureInputs { bool isInout = false; if (auto inoutType = dyn_cast(substType)) { - isInout = true; + // The `self` parameter of convenience initializers is considered 'inout' + // for semantic purposes in the AST, but at the ABI level is always + // passed by value in and returned out. + if (!forSelf || rep != SILFunctionTypeRepresentation::ObjCMethod) { + isInout = true; + } substType = inoutType.getObjectType(); origType = origType.getWithoutSpecifierType(); } diff --git a/lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.cpp b/lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.cpp index bc30fb774b207..8fb74b790d9c1 100644 --- a/lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.cpp +++ b/lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.cpp @@ -1540,8 +1540,17 @@ collectDelegatingInitUses(const DIMemoryObjectInfo &TheMemory, // for the dynamic type of that uninitialized object. if (isa(User)) { auto UserVal = cast(User); - if (UserVal->hasOneUse() - && isa(UserVal->getSingleUse()->get())) { + bool onlyUseIsValueMetatype = true; + for (auto use : UserVal->getUses()) { + // A taking or copying load is supposed to destroy the value after + // use, which we can turn into a no-op. + if (isa(use->getUser()) + || isa(use->getUser())) + continue; + onlyUseIsValueMetatype = false; + break; + } + if (onlyUseIsValueMetatype) { Kind = DIUseKind::LoadForTypeOfSelf; } } diff --git a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp index 44edb6eba2ebe..012f8c4de55e1 100644 --- a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp +++ b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp @@ -857,7 +857,17 @@ void LifetimeChecker::handleLoadForTypeOfSelfUse(const DIMemoryUse &Use) { break; } assert(valueMetatype); - auto metatypeArgument = load->getFunction()->getSelfMetadataArgument(); + SILValue metatypeArgument = load->getFunction()->getSelfMetadataArgument(); + + // Bitcast to the formal metatype of the self argument, which may be + // different in @dynamic_self-ness. + if (metatypeArgument->getType() != valueMetatype->getType()) { + SILBuilderWithScope B(valueMetatype); + metatypeArgument = B.createUncheckedBitCast(valueMetatype->getLoc(), + metatypeArgument, + valueMetatype->getType()); + } + replaceAllSimplifiedUsesAndErase(valueMetatype, metatypeArgument, [](SILInstruction*) { }); } diff --git a/test/SILGen/objc_convenience_init.swift b/test/SILGen/objc_convenience_init.swift new file mode 100644 index 0000000000000..e549f53ec7e33 --- /dev/null +++ b/test/SILGen/objc_convenience_init.swift @@ -0,0 +1,13 @@ +// RUN: %target-swift-emit-sil(mock-sdk: %clang-importer-sdk) -swift-version 5 -verify %s + +import Foundation + +class X: NSObject { + override init() {} + + static func foo() -> Self { fatalError() } + + @objc convenience init(x: Int) { + self = type(of: self).foo() + } +} diff --git a/test/SILOptimizer/definite_init_type_of_self_in_convenience_init.swift b/test/SILOptimizer/definite_init_type_of_self_in_convenience_init.swift index d6d3ab4b1bb48..aca0ba71cd25c 100644 --- a/test/SILOptimizer/definite_init_type_of_self_in_convenience_init.swift +++ b/test/SILOptimizer/definite_init_type_of_self_in_convenience_init.swift @@ -1,4 +1,5 @@ -// RUN: %target-swift-emit-sil -verify %s +// RUN: %target-swift-emit-sil -swift-version 4 -verify %s +// RUN: %target-swift-emit-sil -swift-version 5 -verify %s // Integration test to ensure that `type(of: self)` keeps working in // class convenience initializers, even though they are now implemented as diff --git a/test/decl/init/convenience_init_assignment_s4.swift b/test/decl/init/convenience_init_assignment_s4.swift new file mode 100644 index 0000000000000..4f12514974588 --- /dev/null +++ b/test/decl/init/convenience_init_assignment_s4.swift @@ -0,0 +1,26 @@ +// RUN: %target-typecheck-verify-swift -swift-version 4 + +final class A { + static let a = A() + convenience init(a: ()) { + self = A.a // expected-error{{immutable}} + } +} + +private func _forceCastToSelf(_ object: AnyObject) -> T { + return object as! T +} + +class B { + static let b = B() + + convenience init(bWrong: ()) { + self = B.b // expected-error{{immutable}} + } + + convenience init(b: ()) { + self = _forceCastToSelf(B.b) // expected-error{{immutable}} + } +} + +class C: B { } diff --git a/test/decl/init/convenience_init_assignment_s5.swift b/test/decl/init/convenience_init_assignment_s5.swift new file mode 100644 index 0000000000000..46f493820ce6a --- /dev/null +++ b/test/decl/init/convenience_init_assignment_s5.swift @@ -0,0 +1,39 @@ +// RUN: %target-typecheck-verify-swift -swift-version 5 + +final class A { + static let a = A() + convenience init(a: ()) { + self = A.a + } +} + +private func _forceCastToSelf(_ object: AnyObject) -> T { + return object as! T +} + +protocol P {} + +extension P { + static var selfProperty: Self { fatalError() } + static func selfReturningMethod() -> Self { fatalError() } + static func selfLaunderingMethod(_: Self) -> Self { fatalError() } + + init(protocolExt: ()) { fatalError() } +} + +class B: P { + static let b = B() + + convenience init(otherInstance: B) { + self = B.b // expected-error{{cannot assign value of type 'B' to type 'Self'}} + self = otherInstance // expected-error{{cannot assign value of type 'B' to type 'Self'}} + self = _forceCastToSelf(B.b) + self = type(of: self).selfProperty + self = type(of: self).selfReturningMethod() + self = type(of: self).selfLaunderingMethod(otherInstance) // expected-error{{cannot assign value of type 'B' to type 'Self'}} + self = type(of: self).selfLaunderingMethod(type(of: self).selfReturningMethod()) + self = type(of: self).init(protocolExt: ()) + } +} + +class C: B { }