Skip to content

Commit ab54ea7

Browse files
xedinxedin
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.
1 parent 8edf870 commit ab54ea7

8 files changed

+67
-65
lines changed

‎lib/SILGen/SILGenFunction.cpp‎

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

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

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

18471850
capturedArgs.push_back(capturedBase);
18481851
}
18491852

18501853
PartialApplyInst *setterPAI =
18511854
B.createPartialApply(loc, setterFRef, substitutions, capturedArgs,
1852-
ParameterConvention::Direct_Guaranteed);
1855+
ParameterConvention::Direct_Guaranteed,
1856+
PartialApplyInst::OnStackKind::OnStack);
18531857
return emitManagedRValueWithCleanup(setterPAI).getValue();
18541858
}
18551859

‎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)