Skip to content

Commit 52eca57

Browse files
committed
[move-only] Make sure we mark local function inout parameters with mark_must_check.
Originally, we were relying on capture info to determine if we needed to insert this mark_must_check. This ignored that the way that we are handling escaping/non-escaping is something that is approximated in the AST but actually determined at SIL level. With that in mind, rather than relying on the capture info here, just rely on us having an inout argument. The later SIL level checking for inout escapes is able to handle mark_must_check and knows how to turn off noncopyable errors in the closure where we detect the error to prevent us from emitting further errors due to the mark_must_check here. I discovered this while playing with the previous commit. rdar://112555589
1 parent 6675084 commit 52eca57

File tree

3 files changed

+29
-12
lines changed

3 files changed

+29
-12
lines changed

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
}

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: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,17 @@ struct Test : ~Copyable {
2828
e = E()
2929
e2 = E2()
3030
func capture() {
31-
let _ = self // expected-error {{copy of noncopyable typed value}}
32-
let _ = self.e2 // expected-error {{copy of noncopyable typed value}}
31+
let _ = self
32+
let _ = self.e2 // expected-error {{cannot partially consume 'self'}}
33+
}
34+
capture()
35+
}
36+
37+
init(y: ()) { // expected-error {{missing reinitialization of closure capture 'self' after consume}}
38+
e = E()
39+
e2 = E2()
40+
func capture() {
41+
let _ = self // expected-note {{consumed here}}
3342
}
3443
capture()
3544
}

0 commit comments

Comments
 (0)