diff --git a/include/swift/SIL/SILInstruction.h b/include/swift/SIL/SILInstruction.h index 49563105fa0dd..f8967b9de6279 100644 --- a/include/swift/SIL/SILInstruction.h +++ b/include/swift/SIL/SILInstruction.h @@ -8318,8 +8318,8 @@ class MarkMustCheckInst ConsumableAndAssignable, /// A signal to the move only checker to perform no consume or assign - /// checking. This forces the result of this instruction owned value to never - /// be consumed (for let/var semantics) or assigned over (for var + /// checking. This forces the result of this instruction owned value to + /// never be consumed (for let/var semantics) or assigned over (for var /// semantics). Of course, we still allow for non-consuming uses. NoConsumeOrAssign, @@ -8330,6 +8330,11 @@ class MarkMustCheckInst /// uninitialized state), but we are ok with the user assigning a new value, /// completely assigning over the value at once. AssignableButNotConsumable, + + /// A signal to the move checker that the given value cannot be consumed or + /// assigned, but is allowed to be initialized. This is used for situations + /// like class initializers. + InitableButNotConsumable, }; private: @@ -8348,6 +8353,8 @@ class MarkMustCheckInst public: CheckKind getCheckKind() const { return kind; } + void setCheckKind(CheckKind newKind) { kind = newKind; } + bool hasMoveCheckerKind() const { switch (kind) { case CheckKind::Invalid: @@ -8355,6 +8362,7 @@ class MarkMustCheckInst case CheckKind::ConsumableAndAssignable: case CheckKind::NoConsumeOrAssign: case CheckKind::AssignableButNotConsumable: + case CheckKind::InitableButNotConsumable: return true; } } diff --git a/lib/SIL/IR/SILPrinter.cpp b/lib/SIL/IR/SILPrinter.cpp index 6694c198a8413..2077a1b38dda8 100644 --- a/lib/SIL/IR/SILPrinter.cpp +++ b/lib/SIL/IR/SILPrinter.cpp @@ -2010,6 +2010,9 @@ class SILPrinter : public SILInstructionVisitor { case CheckKind::AssignableButNotConsumable: *this << "[assignable_but_not_consumable] "; break; + case CheckKind::InitableButNotConsumable: + *this << "[initable_but_not_consumable] "; + break; } *this << getIDAndType(I->getOperand()); } diff --git a/lib/SIL/Parser/ParseSIL.cpp b/lib/SIL/Parser/ParseSIL.cpp index 5f79fb6e55db8..e84ec8c533ad5 100644 --- a/lib/SIL/Parser/ParseSIL.cpp +++ b/lib/SIL/Parser/ParseSIL.cpp @@ -3712,7 +3712,10 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B, .Case("consumable_and_assignable", CheckKind::ConsumableAndAssignable) .Case("no_consume_or_assign", CheckKind::NoConsumeOrAssign) - .Case("assignable_but_not_consumable", CheckKind::AssignableButNotConsumable) + .Case("assignable_but_not_consumable", + CheckKind::AssignableButNotConsumable) + .Case("initable_but_not_consumable", + CheckKind::InitableButNotConsumable) .Default(CheckKind::Invalid); if (CKind == CheckKind::Invalid) { diff --git a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp index 4e0e1c04da904..9d7a6bb88ce97 100644 --- a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp +++ b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp @@ -2312,7 +2312,6 @@ void LifetimeChecker::updateInstructionForInitState(unsigned UseID) { // If this is an assign, rewrite it based on whether it is an initialization // or not. if (auto *AI = dyn_cast(Inst)) { - // Remove this instruction from our data structures, since we will be // removing it. Use.Inst = nullptr; @@ -2328,6 +2327,24 @@ void LifetimeChecker::updateInstructionForInitState(unsigned UseID) { : AssignOwnershipQualifier::Reassign)); } + // Look and see if we are assigning a moveonly type into a mark_must_check + // [assignable_but_not_consumable]. If we are, then we need to transition + // its flag to initable_but_not_assignable. + // + // NOTE: We should only ever have to do this for a single level since SILGen + // always initializes values completely and we enforce that invariant. + if (InitKind == IsInitialization) { + if (auto *mmci = + dyn_cast(stripAccessMarkers(AI->getDest()))) { + if (mmci->getCheckKind() == + MarkMustCheckInst::CheckKind::AssignableButNotConsumable && + isa(stripAccessMarkers(mmci->getOperand()))) { + mmci->setCheckKind( + MarkMustCheckInst::CheckKind::InitableButNotConsumable); + } + } + } + return; } if (auto *AI = dyn_cast(Inst)) { diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp index b758b9e939f9d..62807316243c3 100644 --- a/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp +++ b/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp @@ -500,8 +500,12 @@ static bool isInOutDefThatNeedsEndOfFunctionLiveness(MarkMustCheckInst *markedAd if (isa(stripAccessMarkers(operand))) return true; + + if (auto *rei = dyn_cast(stripAccessMarkers(operand))) + return true; } + return false; } @@ -2159,6 +2163,19 @@ void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary( insertUndefDebugValue(insertPt); } else { auto *inst = cast(defPair.first); + + // If we have a dead def that is our mark must check and that mark must + // check was an init but not consumable, then do not destroy that + // def. This is b/c we are in some sort of class initialization and we are + // looking at the initial part of the live range before the initialization + // has occured. This is our way of makinmg this fit the model that the + // checker expects (which is that values are always initialized at the def + // point). + if (markedValue && + markedValue->getCheckKind() == + MarkMustCheckInst::CheckKind::InitableButNotConsumable) + continue; + auto *insertPt = inst->getNextInstruction(); assert(insertPt && "def instruction was a terminator"); insertDestroyBeforeInstruction(addressUseState, insertPt, diff --git a/lib/Serialization/DeserializeSIL.cpp b/lib/Serialization/DeserializeSIL.cpp index 4586db1f7f480..a5042ff1ea7ad 100644 --- a/lib/Serialization/DeserializeSIL.cpp +++ b/lib/Serialization/DeserializeSIL.cpp @@ -2200,17 +2200,6 @@ bool SILDeserializer::readSILInstruction(SILFunction *Fn, break; } - case SILInstructionKind::MarkMustCheckInst: { - using CheckKind = MarkMustCheckInst::CheckKind; - auto Ty = MF->getType(TyID); - auto CKind = CheckKind(Attr); - ResultInst = Builder.createMarkMustCheckInst( - Loc, - getLocalValue(ValID, getSILType(Ty, (SILValueCategory)TyCategory, Fn)), - CKind); - break; - } - case SILInstructionKind::MarkUnresolvedReferenceBindingInst: { using Kind = MarkUnresolvedReferenceBindingInst::Kind; auto ty = MF->getType(TyID); @@ -2288,6 +2277,16 @@ bool SILDeserializer::readSILInstruction(SILFunction *Fn, ResultInst = Builder.createMarkUninitialized(Loc, Val, Kind); break; } + case SILInstructionKind::MarkMustCheckInst: { + using CheckKind = MarkMustCheckInst::CheckKind; + auto Ty = MF->getType(TyID); + auto CKind = CheckKind(Attr); + ResultInst = Builder.createMarkMustCheckInst( + Loc, + getLocalValue(ValID, getSILType(Ty, (SILValueCategory)TyCategory, Fn)), + CKind); + break; + } case SILInstructionKind::StoreInst: { auto Ty = MF->getType(TyID); SILType addrType = getSILType(Ty, (SILValueCategory)TyCategory, Fn); diff --git a/lib/Serialization/SerializeSIL.cpp b/lib/Serialization/SerializeSIL.cpp index eeaf3b56ee768..116a97ff413f5 100644 --- a/lib/Serialization/SerializeSIL.cpp +++ b/lib/Serialization/SerializeSIL.cpp @@ -1482,7 +1482,6 @@ void SILSerializer::writeSILInstruction(const SILInstruction &SI) { case SILInstructionKind::CopyValueInst: case SILInstructionKind::ExplicitCopyValueInst: case SILInstructionKind::MoveValueInst: - case SILInstructionKind::MarkMustCheckInst: case SILInstructionKind::MarkUnresolvedReferenceBindingInst: case SILInstructionKind::MoveOnlyWrapperToCopyableValueInst: case SILInstructionKind::CopyableToMoveOnlyWrapperValueInst: @@ -1561,6 +1560,11 @@ void SILSerializer::writeSILInstruction(const SILInstruction &SI) { writeOneOperandLayout(SI.getKind(), Attr, SI.getOperand(0)); break; } + case SILInstructionKind::MarkMustCheckInst: { + unsigned Attr = unsigned(cast(&SI)->getCheckKind()); + writeOneOperandExtraAttributeLayout(SI.getKind(), Attr, SI.getOperand(0)); + break; + } case SILInstructionKind::MarkUninitializedInst: { unsigned Attr = (unsigned)cast(&SI)->getMarkUninitializedKind(); diff --git a/test/Interpreter/moveonly.swift b/test/Interpreter/moveonly.swift index 5d1c88a5a6e51..f50a159a5f6d8 100644 --- a/test/Interpreter/moveonly.swift +++ b/test/Interpreter/moveonly.swift @@ -10,40 +10,82 @@ var Tests = TestSuite("MoveOnlyTests") @_moveOnly struct FD { - var a = LifetimeTracked(0) + var a = LifetimeTracked(0) - deinit { - } + deinit { + } } Tests.test("simple deinit called once") { - do { - let s = FD() - } - expectEqual(0, LifetimeTracked.instances) + do { + let s = FD() + } + expectEqual(0, LifetimeTracked.instances) } Tests.test("ref element addr destroyed once") { - class CopyableKlass { - var fd = FD() - } + class CopyableKlass { + var fd = FD() + } - func assignCopyableKlass(_ x: CopyableKlass) { - x.fd = FD() - } + func assignCopyableKlass(_ x: CopyableKlass) { + x.fd = FD() + } - do { - let x = CopyableKlass() - assignCopyableKlass(x) - } - expectEqual(0, LifetimeTracked.instances) + do { + let x = CopyableKlass() + assignCopyableKlass(x) + } + expectEqual(0, LifetimeTracked.instances) } var global = FD() Tests.test("global destroyed once") { - do { - global = FD() + do { + global = FD() + } + expectEqual(0, LifetimeTracked.instances) +} + +// TODO (rdar://107494072): Move-only types with deinits declared inside +// functions sometimes lose their deinit function. +// When that's fixed, FD2 can be moved back inside the test closure below. +@_moveOnly +struct FD2 { + var field = 5 + static var count = 0 + init() { FD2.count += 1 } + deinit { + FD2.count -= 1 + print("In deinit!") + } + func use() {} +} + +Tests.test("deinit not called in init when assigned") { + class FDHaver { + var fd: FD2 + + init() { + self.fd = FD2() } - expectEqual(0, LifetimeTracked.instances) + } + + class FDHaver2 { + var fd: FD2 + + init() { + self.fd = FD2() + self.fd = FD2() + self.fd = FD2() + self.fd.use() + } + } + + do { + let haver = FDHaver2() + let _ = haver + } + expectEqual(0, FD2.count) } diff --git a/test/SILOptimizer/moveonly_class_inits_end_to_end.swift b/test/SILOptimizer/moveonly_class_inits_end_to_end.swift new file mode 100644 index 0000000000000..a6998a5199d32 --- /dev/null +++ b/test/SILOptimizer/moveonly_class_inits_end_to_end.swift @@ -0,0 +1,31 @@ +// RUN: %target-swift-frontend -module-name moveonly_class_inits_end_to_end %s -emit-sil -o - | %FileCheck %s + +// A test that makes sure end to end in a copyable class containing a +// non-copyable type, in the init, we only have a single destroy_addr. + +@_moveOnly +public struct MO { + var x: Int8 = 0 + deinit { print("destroyed MO") } +} + +public class MOHaver { + var mo: MO + + // CHECK-LABEL: sil hidden @$s028moveonly_class_inits_end_to_D07MOHaverCACycfc : $@convention(method) (@owned MOHaver) -> @owned MOHaver { + // CHECK: bb0([[ARG:%.*]] : + // CHECK-NEXT: debug_value [[ARG]] + // CHECK-NEXT: [[META:%.*]] = metatype + // CHECK-NEXT: function_ref MO.init() + // CHECK-NEXT: [[FUNC:%.*]] = function_ref @$s028moveonly_class_inits_end_to_D02MOVACycfC : + // CHECK-NEXT: [[RESULT:%.*]] = apply [[FUNC]]([[META]]) + // CHECK-NEXT: [[REF:%.*]] = ref_element_addr [[ARG]] + // CHECK-NEXT: [[REF_ACCESS:%.*]] = begin_access [modify] [dynamic] [[REF]] + // CHECK-NEXT: store [[RESULT]] to [[REF_ACCESS]] + // CHECK-NEXT: end_access [[REF_ACCESS]] + // CHECK-NEXT: return [[ARG]] + // CHECK-NEXT: } // end sil function '$s028moveonly_class_inits_end_to_D07MOHaverCACycfc' + init() { + self.mo = MO() + } +}