Skip to content

Commit 6675084

Browse files
gottesmmgottesmm
committed
[move-only] Do not try to capture self as an immutable box.
The problem here is that the logic was conditionalized on all noncopyable parameters that are borrowed as having the ValueOwnership::Shared flag set. This is only true for user written parameters. Implicit noncopyable parameters like self do not have ValueOwnership::Shared set upon them. We could potentially do that in Sema, but Sema does not know what the proper convention of self is since that information is in TypeLowering today. With that in mind, conditionalize the logic here so we do the right thing. rdar://112547982
1 parent 34014ec commit 6675084

File tree

3 files changed

+185
-2
lines changed

3 files changed

+185
-2
lines changed

‎lib/SIL/IR/TypeLowering.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ CaptureKind TypeConverter::getDeclCaptureKind(CapturedValue capture,
126126
if (!var->supportsMutation() && lowering.getLoweredType().isPureMoveOnly() &&
127127
!capture.isNoEscape()) {
128128
auto *param = dyn_cast<ParamDecl>(var);
129-
if (!param || param->getValueOwnership() != ValueOwnership::Shared) {
129+
if (!param || (param->getValueOwnership() != ValueOwnership::Shared &&
130+
!param->isSelfParameter())) {
130131
return CaptureKind::ImmutableBox;
131132
}
132133
}

‎test/SILGen/moveonly.swift

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2403,7 +2403,7 @@ public func testSubscriptReadModify_BaseLoadable_ResultAddressOnly_InOut(m: inou
24032403
}
24042404

24052405
// CHECK-LABEL: sil [ossa] @$s8moveonly61testSubscriptReadModify_BaseLoadable_ResultAddressOnly_GlobalyyF : $@convention(thin) () -> () {
2406-
// CHECK: [[GLOBAL_ADDR:%.*]] = global_addr @$s8moveonly39globalLoadableSubscriptReadModifyTesterAA0cdefG0Vvp :
2406+
// CHECK: [[GLOBAL_ADDR:%.*]] = global_addr @$s8moveonly39globalLoadableSubscriptReadModifyTesterAA0cdefG0Vvp :
24072407
//
24082408
// The get call
24092409
// CHECK: [[ACCESS:%.*]] = begin_access [read] [dynamic] [[GLOBAL_ADDR]]
@@ -3117,3 +3117,96 @@ public func testSubscriptGetModifyThroughParentClass_BaseLoadable_ResultAddressO
31173117
m.computedTester2[0].mutatingFunc()
31183118
}
31193119

3120+
//////////////////////////////
3121+
// MARK: Capture Self Tests //
3122+
//////////////////////////////
3123+
3124+
func testSelfCaptureHandledCorrectly() {
3125+
struct E {
3126+
var a: [Int] = []
3127+
}
3128+
3129+
struct Test : ~Copyable {
3130+
var e: E
3131+
3132+
// Test that we capture inits by address.
3133+
//
3134+
// CHECK-LABEL: sil private [ossa] @$s8moveonly31testSelfCaptureHandledCorrectlyyyF4TestL_VADycfC : $@convention(method) (@thin Test.Type) -> @owned Test {
3135+
// CHECK: [[BOX:%.*]] = alloc_box ${ var Test }
3136+
// CHECK: [[MARK_UNINIT:%.*]] = mark_uninitialized [rootself] [[BOX]]
3137+
// CHECK: [[BORROW:%.*]] = begin_borrow [lexical] [[MARK_UNINIT]]
3138+
// CHECK: [[PROJECT:%.*]] = project_box [[BORROW]]
3139+
// CHECK: apply {{%.*}}([[PROJECT]])
3140+
// CHECK: } // end sil function '$s8moveonly31testSelfCaptureHandledCorrectlyyyF4TestL_VADycfC'
3141+
init() {
3142+
e = E()
3143+
func capture() {
3144+
let e = self.e
3145+
}
3146+
capture()
3147+
}
3148+
3149+
// CHECK-LABEL: sil private [ossa] @$s8moveonly31testSelfCaptureHandledCorrectlyyyF4TestL_V22captureByLocalFunctionyyF : $@convention(method) (@guaranteed Test) -> () {
3150+
// CHECK: bb0([[ARG:%.*]] : @guaranteed $Test):
3151+
// CHECK: [[COPY:%.*]] = copy_value [[ARG]]
3152+
// CHECK: [[MARK:%.*]] = mark_must_check [no_consume_or_assign] [[COPY]]
3153+
// CHECK: [[BORROW:%.*]] = begin_borrow [[MARK]]
3154+
// CHECK: apply {{%.*}}([[BORROW]])
3155+
// CHECK: end_borrow [[BORROW]]
3156+
// CHECK: destroy_value [[MARK]]
3157+
// CHECK: } // end sil function '$s8moveonly31testSelfCaptureHandledCorrectlyyyF4TestL_V22captureByLocalFunctionyyF'
3158+
func captureByLocalFunction() {
3159+
func capture() {
3160+
let e = self.e
3161+
}
3162+
capture()
3163+
}
3164+
3165+
// CHECK-LABEL: sil private [ossa] @$s8moveonly31testSelfCaptureHandledCorrectlyyyF4TestL_V17captureByLocalLetyyF : $@convention(method) (@guaranteed Test) -> () {
3166+
// CHECK: bb0([[ARG:%.*]] : @guaranteed $Test):
3167+
// CHECK: [[COPY:%.*]] = copy_value [[ARG]]
3168+
// CHECK: [[MARK:%.*]] = mark_must_check [no_consume_or_assign] [[COPY]]
3169+
// CHECK: [[COPY2:%.*]] = copy_value [[MARK]]
3170+
// CHECK: [[PAI:%.*]] = partial_apply [callee_guaranteed] {{%.*}}([[COPY2]])
3171+
// CHECK: destroy_value [[MARK]]
3172+
// CHECK: } // end sil function '$s8moveonly31testSelfCaptureHandledCorrectlyyyF4TestL_V17captureByLocalLetyyF'
3173+
func captureByLocalLet() {
3174+
let f = {
3175+
let e = self.e
3176+
}
3177+
3178+
f()
3179+
}
3180+
3181+
// CHECK-LABEL: sil private [ossa] @$s8moveonly31testSelfCaptureHandledCorrectlyyyF4TestL_V17captureByLocalVaryyF : $@convention(method) (@guaranteed Test) -> () {
3182+
// CHECK: bb0([[ARG:%.*]] : @guaranteed $Test):
3183+
// CHECK: [[COPY:%.*]] = copy_value [[ARG]]
3184+
// CHECK: [[MARK:%.*]] = mark_must_check [no_consume_or_assign] [[COPY]]
3185+
// CHECK: [[COPY2:%.*]] = copy_value [[MARK]]
3186+
// CHECK: [[PAI:%.*]] = partial_apply [callee_guaranteed] {{%.*}}([[COPY2]])
3187+
// CHECK: destroy_value [[MARK]]
3188+
// CHECK: } // end sil function '$s8moveonly31testSelfCaptureHandledCorrectlyyyF4TestL_V17captureByLocalVaryyF'
3189+
func captureByLocalVar() {
3190+
var f = {}
3191+
f = {
3192+
let e = self.e
3193+
}
3194+
f()
3195+
}
3196+
3197+
// CHECK-LABEL: sil private [ossa] @$s8moveonly31testSelfCaptureHandledCorrectlyyyF4TestL_V27captureByNonEscapingClosureyyF : $@convention(method) (@guaranteed Test) -> () {
3198+
// CHECK: bb0([[ARG:%.*]] : @guaranteed $Test):
3199+
// CHECK: [[COPY:%.*]] = copy_value [[ARG]]
3200+
// CHECK: [[MARK:%.*]] = mark_must_check [no_consume_or_assign] [[COPY]]
3201+
// CHECK: [[COPY2:%.*]] = copy_value [[MARK]]
3202+
// CHECK: [[PAI:%.*]] = partial_apply [callee_guaranteed] {{%.*}}([[COPY2]])
3203+
// CHECK: destroy_value [[MARK]]
3204+
// CHECK: } // end sil function '$s8moveonly31testSelfCaptureHandledCorrectlyyyF4TestL_V27captureByNonEscapingClosureyyF'
3205+
func captureByNonEscapingClosure() {
3206+
func useClosure(_ f: () -> ()) {}
3207+
useClosure {
3208+
let e = self.e
3209+
}
3210+
}
3211+
}
3212+
}

‎test/SILOptimizer/moveonly_self_captures.swift

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// RUN: %target-swift-emit-sil -sil-verify-all -verify %s
2+
3+
class Klass {}
4+
5+
struct E {
6+
var k = Klass()
7+
}
8+
9+
struct E2 : ~Copyable {
10+
var k = Klass()
11+
}
12+
13+
struct Test : ~Copyable {
14+
var e: E
15+
var e2: E2
16+
17+
// Test that we capture inits by address.
18+
init() {
19+
e = E()
20+
e2 = E2()
21+
func capture() {
22+
let _ = self.e
23+
}
24+
capture()
25+
}
26+
27+
init(x: ()) {
28+
e = E()
29+
e2 = E2()
30+
func capture() {
31+
let _ = self // expected-error {{copy of noncopyable typed value}}
32+
let _ = self.e2 // expected-error {{copy of noncopyable typed value}}
33+
}
34+
capture()
35+
}
36+
37+
func captureByLocalFunction() {
38+
func capture() {
39+
let _ = self.e
40+
}
41+
capture()
42+
}
43+
44+
func captureByLocalFunction2() { // expected-error {{noncopyable 'self' cannot be consumed when captured by an escaping closure}}
45+
func capture() {
46+
let _ = self.e2 // expected-note {{consumed here}}
47+
}
48+
capture()
49+
}
50+
51+
func captureByLocalFunction3() { // expected-error {{noncopyable 'self' cannot be consumed when captured by an escaping closure}}
52+
func capture() {
53+
let _ = self // expected-note {{consumed here}}
54+
}
55+
capture()
56+
}
57+
58+
func captureByLocalLet() { // expected-error {{'self' cannot be captured by an escaping closure since it is a borrowed parameter}}
59+
let f = { // expected-note {{capturing 'self' here}}
60+
let _ = self.e
61+
}
62+
63+
f()
64+
}
65+
66+
func captureByLocalVar() { // expected-error {{'self' cannot be captured by an escaping closure since it is a borrowed parameter}}
67+
var f = {}
68+
f = { // expected-note {{closure capturing 'self' here}}
69+
let _ = self.e
70+
}
71+
f()
72+
}
73+
74+
func captureByNonEscapingClosure() {
75+
func useClosure(_ f: () -> ()) {}
76+
useClosure {
77+
let _ = self.e
78+
}
79+
}
80+
81+
func captureByNonEscapingClosure2() { // expected-error {{'self' cannot be consumed when captured by an escaping closure}}
82+
func useClosure(_ f: () -> ()) {}
83+
useClosure {
84+
let _ = self // expected-note {{consumed here}}
85+
}
86+
}
87+
}
88+
89+

0 commit comments

Comments
 (0)