Skip to content

Commit 500c14b

Browse files
committed
[SILGen] InitAccessors: Prevent setter closure from escaping
The closure with applied base is not escaping and gets applied only once (when self is fully initialized). Let's make sure that the partial application results in on-stack closure that borrows "self" instead of copying it. (cherry picked from commit ab54ea7)
1 parent a66f28e commit 500c14b

8 files changed

+111
-63
lines changed

lib/SILGen/SILGenFunction.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1820,8 +1820,8 @@ SILGenFunction::emitApplyOfSetterToBase(SILLocation loc, SILDeclRef setter,
18201820
SmallVector<ManagedValue, 4> captures;
18211821
emitCaptures(loc, setter, CaptureEmission::AssignByWrapper, captures);
18221822

1823-
for (auto capture : captures)
1824-
capturedArgs.push_back(capture.forward(*this));
1823+
llvm::transform(captures, std::back_inserter(capturedArgs),
1824+
[](auto &capture) { return capture.getValue(); });
18251825
} else {
18261826
assert(base);
18271827

@@ -1839,16 +1839,20 @@ SILGenFunction::emitApplyOfSetterToBase(SILLocation loc, SILDeclRef setter,
18391839
// nonmutating setter, for example.
18401840
capturedBase = B.createTrivialLoadOr(loc, base.getValue(),
18411841
LoadOwnershipQualifier::Copy);
1842+
// On-stack partial apply doesn't take ownership of the base, so
1843+
// we have to destroy it manually.
1844+
enterDestroyCleanup(capturedBase);
18421845
} else {
1843-
capturedBase = base.copy(*this, loc).forward(*this);
1846+
capturedBase = base.borrow(*this, loc).getValue();
18441847
}
18451848

18461849
capturedArgs.push_back(capturedBase);
18471850
}
18481851

18491852
PartialApplyInst *setterPAI =
18501853
B.createPartialApply(loc, setterFRef, substitutions, capturedArgs,
1851-
ParameterConvention::Direct_Guaranteed);
1854+
ParameterConvention::Direct_Guaranteed,
1855+
PartialApplyInst::OnStackKind::OnStack);
18521856
return emitManagedRValueWithCleanup(setterPAI).getValue();
18531857
}
18541858

test/SILGen/objc_properties.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -289,11 +289,11 @@ class SomeWrapperTests {
289289

290290
// CHECK-LABEL: sil hidden [ossa] @$s15objc_properties16SomeWrapperTestsCyACSScfc : $@convention(method) (@owned String, @owned SomeWrapperTests) -> @owned SomeWrapperTests {
291291
// CHECK: [[M:%.*]] = function_ref @$s15objc_properties16SomeWrapperTestsC04someD0SivsTD
292-
// CHECK: [[C:%.*]] = partial_apply [callee_guaranteed] [[M]]({{.*}})
293-
// CHECK: assign_by_wrapper {{%.*}}: $Int to {{%.*}} : $*SomeWrapper, init {{.*}} : $@callee_guaranteed (Int) -> SomeWrapper, set [[C]] : $@callee_guaranteed (Int) -> ()
292+
// CHECK: [[C:%.*]] = partial_apply [callee_guaranteed] [on_stack] [[M]]({{.*}})
293+
// CHECK: assign_by_wrapper {{%.*}}: $Int to {{%.*}} : $*SomeWrapper, init {{.*}} : $@callee_guaranteed (Int) -> SomeWrapper, set [[C]] : $@noescape @callee_guaranteed (Int) -> ()
294294
// CHECK: [[M:%.*]] = function_ref @$s15objc_properties16SomeWrapperTestsC1sSSSgvsTD
295-
// CHECK: [[C:%.*]] = partial_apply [callee_guaranteed] [[M]](
296-
// CHECK: assign_by_wrapper {{.*}} : $Optional<String> to {{.*}} : $*W<Optional<String>>, init {{.*}} : $@callee_guaranteed (@owned Optional<String>) -> @owned W<Optional<String>>, set [[C]] : $@callee_guaranteed (@owned Optional<String>) -> ()
295+
// CHECK: [[C:%.*]] = partial_apply [callee_guaranteed] [on_stack] [[M]](
296+
// CHECK: assign_by_wrapper {{.*}} : $Optional<String> to {{.*}} : $*W<Optional<String>>, init {{.*}} : $@callee_guaranteed (@owned Optional<String>) -> @owned W<Optional<String>>, set [[C]] : $@noescape @callee_guaranteed (@owned Optional<String>) -> ()
297297
init(_ s: String) {
298298
someWrapper = 1000
299299
self.s = s

test/SILGen/property_wrapper_local.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ func testLocalWrapper() {
2525
// CHECK: [[S:%.*]] = function_ref @$s22property_wrapper_local16testLocalWrapperyyF5valueL_Sivs : $@convention(thin) (Int, @guaranteed { var Wrapper<Int> }) -> ()
2626
// CHECK-NEXT: [[C:%.*]] = copy_value [[W]] : ${ var Wrapper<Int> }
2727
// CHECK-NOT: mark_function_escape
28-
// CHECK-NEXT: [[SPA:%.*]] = partial_apply [callee_guaranteed] [[S]]([[C]]) : $@convention(thin) (Int, @guaranteed { var Wrapper<Int> }) -> ()
29-
// CHECK-NEXT: assign_by_wrapper {{%.*}} : $Int to [[P]] : $*Wrapper<Int>, init [[IPA]] : $@callee_guaranteed (Int) -> Wrapper<Int>, set [[SPA]] : $@callee_guaranteed (Int) -> ()
28+
// CHECK-NEXT: [[SPA:%.*]] = partial_apply [callee_guaranteed] [on_stack] [[S]]([[C]]) : $@convention(thin) (Int, @guaranteed { var Wrapper<Int> }) -> ()
29+
// CHECK-NEXT: assign_by_wrapper {{%.*}} : $Int to [[P]] : $*Wrapper<Int>, init [[IPA]] : $@callee_guaranteed (Int) -> Wrapper<Int>, set [[SPA]] : $@noescape @callee_guaranteed (Int) -> ()
3030

3131
_ = value
3232
// CHECK: mark_function_escape [[P]] : $*Wrapper<Int>

test/SILGen/property_wrappers.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -903,8 +903,8 @@ struct NonMutatingSetterWrapper<Value> {
903903
struct NonMutatingWrapperTestStruct {
904904
// CHECK-LABEL: sil hidden [ossa] @$s17property_wrappers28NonMutatingWrapperTestStructV3valACSi_tcfC : $@convention(method) (Int, @thin NonMutatingWrapperTestStruct.Type) -> NonMutatingWrapperTestStruct {
905905
// CHECK: %[[LOAD:[0-9]+]] = load [trivial] %[[SRC:[0-9]+]] : $*NonMutatingWrapperTestStruct
906-
// CHECK-NEXT: %[[SET_PA:[0-9]+]] = partial_apply [callee_guaranteed] %[[PW_SETTER:[0-9]+]](%[[LOAD]]) : $@convention(method) (Int, NonMutatingWrapperTestStruct) -> ()
907-
// CHECK-NEXT: assign_by_wrapper %[[SETVAL:[0-9]+]] : $Int to %[[ADDR:[0-9]+]] : $*NonMutatingSetterWrapper<Int>, init %[[INIT_PA:[0-9]+]] : $@callee_guaranteed (Int) -> NonMutatingSetterWrapper<Int>, set %[[SET_PA]] : $@callee_guaranteed (Int) -> ()
906+
// CHECK-NEXT: %[[SET_PA:[0-9]+]] = partial_apply [callee_guaranteed] [on_stack] %[[PW_SETTER:[0-9]+]](%[[LOAD]]) : $@convention(method) (Int, NonMutatingWrapperTestStruct) -> ()
907+
// CHECK-NEXT: assign_by_wrapper %[[SETVAL:[0-9]+]] : $Int to %[[ADDR:[0-9]+]] : $*NonMutatingSetterWrapper<Int>, init %[[INIT_PA:[0-9]+]] : $@callee_guaranteed (Int) -> NonMutatingSetterWrapper<Int>, set %[[SET_PA]] : $@noescape @callee_guaranteed (Int) -> ()
908908
@NonMutatingSetterWrapper var SomeProp: Int
909909
init(val: Int) {
910910
SomeProp = val

test/SILGen/resilient_assign_by_wrapper.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ public class AddressOnlySetter {
3333

3434
// CHECK: [[E2:%.*]] = alloc_stack $AddressOnlyEnum
3535
// CHECK-NEXT: inject_enum_addr [[E2]] : $*AddressOnlyEnum, #AddressOnlyEnum.some!enumelt
36-
// CHECK: [[S:%.*]] = partial_apply [callee_guaranteed] {{%.*}}({{%.*}}) : $@convention(method) (@in AddressOnlyEnum, @guaranteed AddressOnlySetter) -> ()
36+
// CHECK: [[S:%.*]] = partial_apply [callee_guaranteed] [on_stack] {{%.*}}({{%.*}}) : $@convention(method) (@in AddressOnlyEnum, @guaranteed AddressOnlySetter) -> ()
3737
// CHECK: assign_by_wrapper [[E2]] : $*AddressOnlyEnum
38-
// CHECK-SAME: set [[S]] : $@callee_guaranteed (@in AddressOnlyEnum) -> ()
38+
// CHECK-SAME: set [[S]] : $@noescape @callee_guaranteed (@in AddressOnlyEnum) -> ()
3939
self.value = .some
4040
}
4141

@@ -60,7 +60,7 @@ extension SubstitutedSetter where T == Bool {
6060
// CHECK: [[B:%.*]] = alloc_stack $Bool
6161
// CHECK: assign_by_wrapper [[B]] : $*Bool to [[W]] : $*WrapGod<Bool>
6262
// CHECK-SAME: init {{%.*}} : $@callee_guaranteed (@in Bool) -> @out WrapGod<Bool>
63-
// CHECK-SAME: set {{%.*}} : $@callee_guaranteed (@in Bool) -> ()
63+
// CHECK-SAME: set {{%.*}} : $@noescape @callee_guaranteed (@in Bool) -> ()
6464
self.value = true
6565
}
6666
}
@@ -96,7 +96,7 @@ extension ObjectifiedSetter where T == SomeObject {
9696
// CHECK: [[STORAGE:%.*]] = struct_element_addr {{%.*}} : $*ObjectifiedSetter<SomeObject>, #ObjectifiedSetter._value
9797
// CHECK: assign_by_wrapper [[OBJ]] : $SomeObject to [[STORAGE]] : $*WrapGod<SomeObject>
9898
// CHECK-SAME: init {{%.*}} : $@callee_guaranteed (@owned SomeObject) -> @out WrapGod<SomeObject>
99-
// CHECK-SAME: set {{%.*}} : $@callee_guaranteed (@owned SomeObject) -> ()
99+
// CHECK-SAME: set {{%.*}} : $@noescape @callee_guaranteed (@owned SomeObject) -> ()
100100
self.value = SomeObject()
101101
}
102102
}

0 commit comments

Comments
 (0)