Skip to content

Commit 7aea9d4

Browse files
gottesmmgottesmm
authored
Merge pull request #67414 from gottesmm/rdar112555589-112547982
[move-only] Fix two small errors around handling capturing of noncopyable self by local functions
2 parents db20b55 + b060e5b commit 7aea9d4

File tree

6 files changed

+233
-12
lines changed

6 files changed

+233
-12
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
}

‎lib/SILGen/SILGenProlog.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,11 +1141,14 @@ static void emitCaptureArguments(SILGenFunction &SGF,
11411141
fArg->setClosureCapture(true);
11421142
arg = SILValue(fArg);
11431143

1144-
// If our capture is no escape and we have a noncopyable value, insert a
1145-
// consumable and assignable. If we have an escaping closure, we are going
1146-
// to emit an error later in SIL since it is illegal to capture an inout
1147-
// value in an escaping closure.
1148-
if (isInOut && ty.isPureMoveOnly() && capture.isNoEscape()) {
1144+
// If we have an inout noncopyable paramter, insert a consumable and
1145+
// assignable.
1146+
//
1147+
// NOTE: If we have an escaping closure, we are going to emit an error later
1148+
// in SIL since it is illegal to capture an inout value in an escaping
1149+
// closure. The later code knows how to handle that we have the
1150+
// mark_must_check here.
1151+
if (isInOut && ty.isPureMoveOnly()) {
11491152
arg = SGF.B.createMarkMustCheckInst(
11501153
Loc, arg, MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
11511154
}

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2323,6 +2323,18 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
23232323
}
23242324
}
23252325

2326+
// If our partial apply takes this parameter as an inout parameter and it
2327+
// has the no move only diagnostics marker on it, do not emit an error
2328+
// either.
2329+
if (auto *f = pas->getCalleeFunction()) {
2330+
if (f->hasSemanticsAttr(semantics::NO_MOVEONLY_DIAGNOSTICS)) {
2331+
if (ApplySite(pas).getArgumentOperandConvention(*op).isInoutConvention()) {
2332+
diagnosticEmitter.emitEarlierPassEmittedDiagnostic(markedValue);
2333+
return false;
2334+
}
2335+
}
2336+
}
2337+
23262338
if (pas->isOnStack() ||
23272339
ApplySite(pas).getArgumentConvention(*op).isInoutConvention()) {
23282340
LLVM_DEBUG(llvm::dbgs() << "Found on stack partial apply or inout usage!\n");

‎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/SILGen/moveonly_escaping_closure.swift

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,8 @@ func testConsumingEscapeClosureCaptureLet(_ f: consuming @escaping () -> ()) {
614614
// CHECK: } // end sil function '$s16moveonly_closure29testGlobalClosureCaptureInOutyyAA9SingleEltVzF'
615615

616616
// CHECK-LABEL: sil private [ossa] @$s16moveonly_closure29testGlobalClosureCaptureInOutyyAA9SingleEltVzFyycfU_ : $@convention(thin) (@inout_aliasable SingleElt) -> () {
617-
// CHECK: bb0([[PROJECT:%.*]] : @closureCapture
617+
// CHECK: bb0([[ARG:%.*]] : @closureCapture
618+
// CHECK: [[PROJECT:%.*]] = mark_must_check [consumable_and_assignable] [[ARG]]
618619
//
619620
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
620621
// CHECK: [[LOADED:%.*]] = load [copy] [[ACCESS]]
@@ -657,7 +658,8 @@ func testGlobalClosureCaptureInOut(_ x: inout SingleElt) {
657658
// CHECK: } // end sil function '$s16moveonly_closure31testLocalLetClosureCaptureInOutyyAA9SingleEltVzF'
658659
//
659660
// CHECK-LABEL: sil private [ossa] @$s16moveonly_closure31testLocalLetClosureCaptureInOutyyAA9SingleEltVzFyycfU_ : $@convention(thin) (@inout_aliasable SingleElt) -> () {
660-
// CHECK: bb0([[PROJECT:%.*]] : @closureCapture
661+
// CHECK: bb0([[ARG:%.*]] : @closureCapture
662+
// CHECK: [[PROJECT:%.*]] = mark_must_check [consumable_and_assignable] [[ARG]]
661663
//
662664
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
663665
// CHECK: [[LOADED:%.*]] = load [copy] [[ACCESS]]
@@ -704,7 +706,8 @@ func testLocalLetClosureCaptureInOut(_ x: inout SingleElt) {
704706
// CHECK: } // end sil function '$s16moveonly_closure31testLocalVarClosureCaptureInOutyyAA9SingleEltVzF'
705707
//
706708
// CHECK-LABEL: sil private [ossa] @$s16moveonly_closure31testLocalVarClosureCaptureInOutyyAA9SingleEltVzFyycfU_ : $@convention(thin) (@inout_aliasable SingleElt) -> () {
707-
// CHECK: bb0([[PROJECT:%.*]] : @closureCapture
709+
// CHECK: bb0([[ARG:%.*]] : @closureCapture
710+
// CHECK: [[PROJECT:%.*]] = mark_must_check [consumable_and_assignable] [[ARG]]
708711
//
709712
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
710713
// CHECK: [[LOADED:%.*]] = load [copy] [[ACCESS]]
@@ -752,7 +755,8 @@ func testLocalVarClosureCaptureInOut(_ x: inout SingleElt) {
752755
// CHECK: } // end sil function '$s16moveonly_closure026testInOutVarClosureCapturedE0yyyycz_AA9SingleEltVztF'
753756
//
754757
// CHECK-LABEL: sil private [ossa] @$s16moveonly_closure026testInOutVarClosureCapturedE0yyyycz_AA9SingleEltVztFyycfU_ : $@convention(thin) (@inout_aliasable SingleElt) -> () {
755-
// CHECK: bb0([[PROJECT:%.*]] : @closureCapture
758+
// CHECK: bb0([[ARG:%.*]] : @closureCapture
759+
// CHECK: [[PROJECT:%.*]] = mark_must_check [consumable_and_assignable] [[ARG]]
756760
//
757761
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
758762
// CHECK: [[LOADED:%.*]] = load [copy] [[ACCESS]]
@@ -807,7 +811,8 @@ func testInOutVarClosureCaptureInOut(_ f: inout () -> (), _ x: inout SingleElt)
807811
// CHECK: } // end sil function '$s16moveonly_closure38testConsumingEscapeClosureCaptureInOutyyyycn_AA9SingleEltVztF'
808812
//
809813
// CHECK-LABEL: sil private [ossa] @$s16moveonly_closure38testConsumingEscapeClosureCaptureInOutyyyycn_AA9SingleEltVztFyycfU_ : $@convention(thin) (@inout_aliasable SingleElt) -> () {
810-
// CHECK: bb0([[PROJECT:%.*]] : @closureCapture
814+
// CHECK: bb0([[ARG:%.*]] : @closureCapture
815+
// CHECK: [[PROJECT:%.*]] = mark_must_check [consumable_and_assignable] [[ARG]]
811816
//
812817
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
813818
// CHECK: [[LOADED:%.*]] = load [copy] [[ACCESS]]

‎test/SILOptimizer/moveonly_self_captures.swift

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
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+
var g: () -> () = {}
14+
struct Test : ~Copyable {
15+
var e: E
16+
var e2: E2
17+
18+
// Test that we capture inits by address.
19+
init() {
20+
e = E()
21+
e2 = E2()
22+
func capture() {
23+
let _ = self.e
24+
}
25+
capture()
26+
}
27+
28+
init(x: ()) {
29+
e = E()
30+
e2 = E2()
31+
func capture() {
32+
let _ = self
33+
let _ = self.e2 // expected-error {{cannot partially consume 'self'}}
34+
}
35+
capture()
36+
}
37+
38+
init(y: ()) { // expected-error {{missing reinitialization of closure capture 'self' after consume}}
39+
e = E()
40+
e2 = E2()
41+
func capture() {
42+
let _ = self // expected-note {{consumed here}}
43+
}
44+
capture()
45+
}
46+
47+
init(z: ()) {
48+
e = E()
49+
e2 = E2()
50+
func capture() {
51+
let _ = self // expected-note {{captured here}}
52+
}
53+
capture()
54+
g = capture // expected-error {{escaping local function captures mutating 'self' parameter}}
55+
}
56+
57+
func captureByLocalFunction() {
58+
func capture() {
59+
let _ = self.e
60+
}
61+
capture()
62+
}
63+
64+
func captureByLocalFunction2() { // expected-error {{noncopyable 'self' cannot be consumed when captured by an escaping closure}}
65+
func capture() {
66+
let _ = self.e2 // expected-note {{consumed here}}
67+
}
68+
capture()
69+
}
70+
71+
func captureByLocalFunction3() { // expected-error {{noncopyable 'self' cannot be consumed when captured by an escaping closure}}
72+
func capture() {
73+
let _ = self // expected-note {{consumed here}}
74+
}
75+
capture()
76+
}
77+
78+
func captureByLocalLet() { // expected-error {{'self' cannot be captured by an escaping closure since it is a borrowed parameter}}
79+
let f = { // expected-note {{capturing 'self' here}}
80+
let _ = self.e
81+
}
82+
83+
f()
84+
}
85+
86+
func captureByLocalVar() { // expected-error {{'self' cannot be captured by an escaping closure since it is a borrowed parameter}}
87+
var f = {}
88+
f = { // expected-note {{closure capturing 'self' here}}
89+
let _ = self.e
90+
}
91+
f()
92+
}
93+
94+
func captureByNonEscapingClosure() {
95+
func useClosure(_ f: () -> ()) {}
96+
useClosure {
97+
let _ = self.e
98+
}
99+
}
100+
101+
func captureByNonEscapingClosure2() { // expected-error {{'self' cannot be consumed when captured by an escaping closure}}
102+
func useClosure(_ f: () -> ()) {}
103+
useClosure {
104+
let _ = self // expected-note {{consumed here}}
105+
}
106+
}
107+
}

0 commit comments

Comments
 (0)