diff --git a/lib/SILGen/SILGenBuilder.cpp b/lib/SILGen/SILGenBuilder.cpp index 4fd7137a7d638..7186762a2b423 100644 --- a/lib/SILGen/SILGenBuilder.cpp +++ b/lib/SILGen/SILGenBuilder.cpp @@ -980,11 +980,12 @@ ManagedValue SILGenBuilder::createBeginBorrow(SILLocation loc, return ManagedValue::forUnmanaged(newValue); } -ManagedValue SILGenBuilder::createMoveValue(SILLocation loc, - ManagedValue value) { +ManagedValue SILGenBuilder::createMoveValue(SILLocation loc, ManagedValue value, + bool isLexical) { assert(value.isPlusOne(SGF) && "Must be +1 to be moved!"); CleanupCloner cloner(*this, value); - auto *mdi = createMoveValue(loc, value.forward(getSILGenFunction())); + auto *mdi = + createMoveValue(loc, value.forward(getSILGenFunction()), isLexical); return cloner.clone(mdi); } @@ -1027,8 +1028,9 @@ ManagedValue SILGenBuilder::createGuaranteedCopyableToMoveOnlyWrapperValue( ManagedValue SILGenBuilder::createMarkMustCheckInst(SILLocation loc, ManagedValue value, MarkMustCheckInst::CheckKind kind) { - assert((value.isPlusOne(SGF) || value.isLValue()) && - "Argument must be at +1 or be an inout!"); + assert((value.isPlusOne(SGF) || value.isLValue() || + value.getType().isAddress()) && + "Argument must be at +1 or be an address!"); CleanupCloner cloner(*this, value); auto *mdi = SILBuilder::createMarkMustCheckInst( loc, value.forward(getSILGenFunction()), kind); diff --git a/lib/SILGen/SILGenBuilder.h b/lib/SILGen/SILGenBuilder.h index 8e89291a6b411..a7c7cc852d152 100644 --- a/lib/SILGen/SILGenBuilder.h +++ b/lib/SILGen/SILGenBuilder.h @@ -441,7 +441,8 @@ class SILGenBuilder : public SILBuilder { bool isLexical = false); using SILBuilder::createMoveValue; - ManagedValue createMoveValue(SILLocation loc, ManagedValue value); + ManagedValue createMoveValue(SILLocation loc, ManagedValue value, + bool isLexical = false); using SILBuilder::createOwnedMoveOnlyWrapperToCopyableValue; ManagedValue createOwnedMoveOnlyWrapperToCopyableValue(SILLocation loc, diff --git a/lib/SILGen/SILGenProlog.cpp b/lib/SILGen/SILGenProlog.cpp index de1e46f7cf139..076f5a7f25d6a 100644 --- a/lib/SILGen/SILGenProlog.cpp +++ b/lib/SILGen/SILGenProlog.cpp @@ -631,23 +631,23 @@ class ArgumentInitHelper { } void updateArgumentValueForBinding(ManagedValue argrv, SILLocation loc, - ParamDecl *pd, SILValue value, + ParamDecl *pd, const SILDebugVariable &varinfo) { bool calledCompletedUpdate = false; SWIFT_DEFER { assert(calledCompletedUpdate && "Forgot to call completed update along " "all paths or manually turn it off"); }; - auto completeUpdate = [&](SILValue value) -> void { - SGF.B.createDebugValue(loc, value, varinfo); - SGF.VarLocs[pd] = SILGenFunction::VarLoc::get(value); + auto completeUpdate = [&](ManagedValue value) -> void { + SGF.B.createDebugValue(loc, value.getValue(), varinfo); + SGF.VarLocs[pd] = SILGenFunction::VarLoc::get(value.getValue()); calledCompletedUpdate = true; }; // If we do not need to support lexical lifetimes, just return value as the // updated value. if (!SGF.getASTContext().SILOpts.supportsLexicalLifetimes(SGF.getModule())) - return completeUpdate(value); + return completeUpdate(argrv); // Look for the following annotations on the function argument: // - @noImplicitCopy @@ -659,15 +659,15 @@ class ArgumentInitHelper { // we need to use copyable to move only to convert it to its move only // form. if (!isNoImplicitCopy) { - if (!value->getType().isMoveOnly()) { + if (!argrv.getType().isMoveOnly()) { // Follow the normal path. The value's lifetime will be enforced based // on its ownership. - return completeUpdate(value); + return completeUpdate(argrv); } // At this point, we have a noncopyable type. If it is owned, create an // alloc_box for it. - if (value->getOwnershipKind() == OwnershipKind::Owned) { + if (argrv.getOwnershipKind() == OwnershipKind::Owned) { // TODO: Once owned values are mutable, this needs to become mutable. auto boxType = SGF.SGM.Types.getContextBoxTypeForCapture( pd, @@ -696,17 +696,18 @@ class ArgumentInitHelper { // misleading consuming message. We still are able to pass it to // non-escaping closures though since the onstack partial_apply does not // consume the value. - assert(value->getOwnershipKind() == OwnershipKind::Guaranteed); - value = SGF.B.createCopyValue(loc, value); - value = SGF.B.createMarkMustCheckInst( - loc, value, MarkMustCheckInst::CheckKind::NoConsumeOrAssign); - SGF.emitManagedRValueWithCleanup(value); - return completeUpdate(value); + assert(argrv.getOwnershipKind() == OwnershipKind::Guaranteed); + argrv = argrv.copy(SGF, loc); + argrv = SGF.B.createMarkMustCheckInst( + loc, argrv, MarkMustCheckInst::CheckKind::NoConsumeOrAssign); + return completeUpdate(argrv); } - if (value->getType().isTrivial(SGF.F)) { - value = SGF.B.createOwnedCopyableToMoveOnlyWrapperValue(loc, value); - value = SGF.B.createMoveValue(loc, value, /*isLexical=*/true); + if (argrv.getType().isTrivial(SGF.F)) { + SILValue value = SGF.B.createOwnedCopyableToMoveOnlyWrapperValue( + loc, argrv.getValue()); + argrv = SGF.emitManagedRValueWithCleanup(value); + argrv = SGF.B.createMoveValue(loc, argrv, /*isLexical=*/true); // If our argument was owned, we use no implicit copy. Otherwise, we // use no copy. @@ -723,40 +724,35 @@ class ArgumentInitHelper { break; } - value = SGF.B.createMarkMustCheckInst(loc, value, kind); - SGF.emitManagedRValueWithCleanup(value); - return completeUpdate(value); + argrv = SGF.B.createMarkMustCheckInst(loc, argrv, kind); + return completeUpdate(argrv); } - if (value->getOwnershipKind() == OwnershipKind::Guaranteed) { - value = SGF.B.createGuaranteedCopyableToMoveOnlyWrapperValue(loc, value); - value = SGF.B.createCopyValue(loc, value); - value = SGF.B.createMarkMustCheckInst( - loc, value, MarkMustCheckInst::CheckKind::NoConsumeOrAssign); - SGF.emitManagedRValueWithCleanup(value); - return completeUpdate(value); + if (argrv.getOwnershipKind() == OwnershipKind::Guaranteed) { + argrv = SGF.B.createGuaranteedCopyableToMoveOnlyWrapperValue(loc, argrv); + argrv = argrv.copy(SGF, loc); + argrv = SGF.B.createMarkMustCheckInst( + loc, argrv, MarkMustCheckInst::CheckKind::NoConsumeOrAssign); + return completeUpdate(argrv); } - if (value->getOwnershipKind() == OwnershipKind::Owned) { + if (argrv.getOwnershipKind() == OwnershipKind::Owned) { // If we have an owned value, forward it into the mark_must_check to // avoid an extra destroy_value. - value = SGF.B.createOwnedCopyableToMoveOnlyWrapperValue( - loc, argrv.forward(SGF)); - value = SGF.B.createMoveValue(loc, value, true /*is lexical*/); - value = SGF.B.createMarkMustCheckInst( - loc, value, MarkMustCheckInst::CheckKind::ConsumableAndAssignable); - SGF.emitManagedRValueWithCleanup(value); - return completeUpdate(value); + argrv = SGF.B.createOwnedCopyableToMoveOnlyWrapperValue(loc, argrv); + argrv = SGF.B.createMoveValue(loc, argrv, true /*is lexical*/); + argrv = SGF.B.createMarkMustCheckInst( + loc, argrv, MarkMustCheckInst::CheckKind::ConsumableAndAssignable); + return completeUpdate(argrv); } - return completeUpdate(value); + return completeUpdate(argrv); } /// Create a SILArgument and store its value into the given Initialization, /// if not null. void makeArgumentIntoBinding(SILLocation loc, ParamDecl *pd) { ManagedValue argrv = makeArgument(loc, pd); - SILValue value = argrv.getValue(); if (pd->isInOut()) { assert(argrv.getType().isAddress() && "expected inout to be address"); } else if (!pd->isImmutableInFunctionBody()) { @@ -774,33 +770,34 @@ class ArgumentInitHelper { SILDebugVariable varinfo(pd->isImmutableInFunctionBody(), ArgNo); if (!argrv.getType().isAddress()) { // NOTE: We setup SGF.VarLocs[pd] in updateArgumentValueForBinding. - updateArgumentValueForBinding(argrv, loc, pd, value, varinfo); + updateArgumentValueForBinding(argrv, loc, pd, varinfo); return; } - if (auto *allocStack = dyn_cast(value)) { + if (auto *allocStack = dyn_cast(argrv.getValue())) { allocStack->setArgNo(ArgNo); if (SGF.getASTContext().SILOpts.supportsLexicalLifetimes( SGF.getModule()) && - SGF.F.getLifetime(pd, value->getType()).isLexical()) + SGF.F.getLifetime(pd, allocStack->getType()).isLexical()) allocStack->setIsLexical(); - SGF.VarLocs[pd] = SILGenFunction::VarLoc::get(value); + SGF.VarLocs[pd] = SILGenFunction::VarLoc::get(allocStack); return; } - SILValue debugOperand = value; - if (value->getType().isMoveOnly()) { + SILValue debugOperand = argrv.getValue(); + + if (argrv.getType().isMoveOnly()) { switch (pd->getValueOwnership()) { case ValueOwnership::Default: if (pd->isSelfParameter()) { - assert(!isa(value) && + assert(!isa(argrv.getValue()) && "Should not have inserted mark must check inst in EmitBBArgs"); if (!pd->isInOut()) { - value = SGF.B.createMarkMustCheckInst( - loc, value, MarkMustCheckInst::CheckKind::NoConsumeOrAssign); + argrv = SGF.B.createMarkMustCheckInst( + loc, argrv, MarkMustCheckInst::CheckKind::NoConsumeOrAssign); } } else { - if (auto *fArg = dyn_cast(value)) { + if (auto *fArg = dyn_cast(argrv.getValue())) { switch (fArg->getArgumentConvention()) { case SILArgumentConvention::Direct_Guaranteed: case SILArgumentConvention::Direct_Owned: @@ -814,51 +811,51 @@ class ArgumentInitHelper { case SILArgumentConvention::Pack_Out: llvm_unreachable("Should have been handled elsewhere"); case SILArgumentConvention::Indirect_In: - value = SGF.B.createMarkMustCheckInst( - loc, value, + argrv = SGF.B.createMarkMustCheckInst( + loc, argrv, MarkMustCheckInst::CheckKind::ConsumableAndAssignable); break; case SILArgumentConvention::Indirect_In_Guaranteed: - value = SGF.B.createMarkMustCheckInst( - loc, value, MarkMustCheckInst::CheckKind::NoConsumeOrAssign); + argrv = SGF.B.createMarkMustCheckInst( + loc, argrv, MarkMustCheckInst::CheckKind::NoConsumeOrAssign); } } else { - assert(isa(value) && + assert(isa(argrv.getValue()) && "Should have inserted mark must check inst in EmitBBArgs"); } } break; case ValueOwnership::InOut: { - assert(isa(value) - && "Expected mark must check inst with inout to be handled in " - "emitBBArgs earlier"); - auto mark = cast(value); + assert(isa(argrv.getValue()) && + "Expected mark must check inst with inout to be handled in " + "emitBBArgs earlier"); + auto mark = cast(argrv.getValue()); debugOperand = mark->getOperand(); break; } case ValueOwnership::Owned: - value = SGF.B.createMarkMustCheckInst( - loc, value, MarkMustCheckInst::CheckKind::ConsumableAndAssignable); + argrv = SGF.B.createMarkMustCheckInst( + loc, argrv, MarkMustCheckInst::CheckKind::ConsumableAndAssignable); break; case ValueOwnership::Shared: - value = SGF.B.createMarkMustCheckInst( - loc, value, MarkMustCheckInst::CheckKind::NoConsumeOrAssign); + argrv = SGF.B.createMarkMustCheckInst( + loc, argrv, MarkMustCheckInst::CheckKind::NoConsumeOrAssign); break; } } DebugValueInst *debugInst = SGF.B.createDebugValueAddr(loc, debugOperand, varinfo); - - if (value != debugOperand) { - if (auto valueInst = dyn_cast(value)) { + + if (argrv.getValue() != debugOperand) { + if (auto valueInst = dyn_cast(argrv.getValue())) { // Move the debug instruction outside of any marker instruction that might // have been applied to the value, so that analysis doesn't move the // debug_value anywhere it shouldn't be. debugInst->moveBefore(valueInst); } } - SGF.VarLocs[pd] = SILGenFunction::VarLoc::get(value); + SGF.VarLocs[pd] = SILGenFunction::VarLoc::get(argrv.getValue()); } void emitParam(ParamDecl *PD) { diff --git a/test/SILGen/moveonly.swift b/test/SILGen/moveonly.swift index 641a313f9d79c..cd89715370c61 100644 --- a/test/SILGen/moveonly.swift +++ b/test/SILGen/moveonly.swift @@ -988,9 +988,9 @@ func testConditionallyInitializedLet() { consumeVal(x) } -///////////////////////////// -// MARK: AddressOnlySetter // -///////////////////////////// +//////////////////////// +// MARK: Setter Tests // +//////////////////////// struct AddressOnlySetterTester : ~Copyable { var a: AddressOnlyProtocol { @@ -1005,6 +1005,24 @@ struct AddressOnlySetterTester : ~Copyable { } } +public class NonFinalClassTest { + // CHECK: sil hidden [transparent] [ossa] @$s8moveonly17NonFinalClassTestC1xAA19AddressOnlyProtocolVvs : $@convention(method) (@in AddressOnlyProtocol, @guaranteed NonFinalClassTest) -> () { + // CHECK: bb0([[INPUT:%.*]] : $*AddressOnlyProtocol, [[SELF:%.*]] : @guaranteed + // CHECK: [[MARK:%.*]] = mark_must_check [consumable_and_assignable] [[INPUT]] + // CHECK: [[TEMP:%.*]] = alloc_stack $AddressOnlyProtocol + // CHECK: copy_addr [[MARK]] to [init] [[TEMP]] + // CHECK: [[REF:%.*]] = ref_element_addr [[SELF]] + // CHECK: [[ACCESS:%.*]] = begin_access [modify] [dynamic] [[REF]] + // CHECK: [[MARK2:%.*]] = mark_must_check [assignable_but_not_consumable] [[ACCESS]] + // CHECK: copy_addr [take] [[TEMP]] to [[MARK2]] + // CHECK: end_access [[ACCESS]] + // CHECK: dealloc_stack [[TEMP]] + // CHECK: destroy_addr [[MARK]] + // CHECK: } // end sil function '$s8moveonly17NonFinalClassTestC1xAA19AddressOnlyProtocolVvs' + var x: AddressOnlyProtocol + init(y: consuming AddressOnlyProtocol) { self.x = y } +} + ///////////////////// // MARK: Subscript // ///////////////////// @@ -3071,3 +3089,4 @@ public func testSubscriptGetModifyThroughParentClass_BaseLoadable_ResultAddressO m.computedTester2[0].nonMutatingFunc() m.computedTester2[0].mutatingFunc() } + diff --git a/test/SILOptimizer/moveonly_addresschecker_diagnostics.swift b/test/SILOptimizer/moveonly_addresschecker_diagnostics.swift index 1a5f716893b07..d00b480e25e6e 100644 --- a/test/SILOptimizer/moveonly_addresschecker_diagnostics.swift +++ b/test/SILOptimizer/moveonly_addresschecker_diagnostics.swift @@ -4486,3 +4486,14 @@ func testMyEnum() { _ = consume x // expected-note {{consumed again here}} } } + +//////////////////////// +// MARK: Setter Tests // +//////////////////////// + +public class NonFinalCopyableKlassWithMoveOnlyField { + var moveOnlyVarStruct = NonTrivialStruct() + let moveOnlyLetStruct = NonTrivialStruct() + var moveOnlyVarProt = AddressOnlyProtocol() + let moveOnlyLetProt = AddressOnlyProtocol() +}