Skip to content

Commit 2346efd

Browse files
gottesmmgottesmm
committed
[move-only] When emitting accesses to let boxes containing a noncopyable type, always emit mark_must_check.
The reason to do this is that: 1. Otherwise, we do not emit markers when someone attempts to consume the let. We need the no_consume_or_assign to be there. 2. We need to insert assign_but_not_consuming so that DI can properly check lets that are conditionally initialized and convert them to initable_but_not_consuming. I included a full definite_init SIL test that validates that we get the correct codegen after DI in this case and emit the appropriate error as well.
1 parent ef4523c commit 2346efd

File tree

4 files changed

+187
-11
lines changed

4 files changed

+187
-11
lines changed

‎lib/SILGen/SILGenLValue.cpp

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,18 +1102,16 @@ namespace {
11021102
addr = enterAccessScope(SGF, loc, base, addr, getTypeData(),
11031103
getAccessKind(), *Enforcement,
11041104
takeActorIsolation());
1105-
// LValue accesses to a `let` box are only ever going to make through
1106-
// definite initialization if they are initializations, which don't
1107-
// require checking since there's no former value to potentially
1108-
// misuse yet.
1109-
if (!box || box->getOperand()->getType().castTo<SILBoxType>()
1110-
->getLayout()->isMutable()) {
1111-
addr = SGF.B.createMarkMustCheckInst(
1112-
loc, addr,
1113-
isReadAccess(getAccessKind())
1105+
// Mark all move only as having mark_must_check.
1106+
//
1107+
// DISCUSSION: LValue access to let boxes must have a mark_must_check
1108+
// to allow for DI to properly handle delayed initialization of the
1109+
// boxes and convert those to initable_but_not_consumable.
1110+
addr = SGF.B.createMarkMustCheckInst(
1111+
loc, addr,
1112+
isReadAccess(getAccessKind())
11141113
? MarkMustCheckInst::CheckKind::NoConsumeOrAssign
11151114
: MarkMustCheckInst::CheckKind::AssignableButNotConsumable);
1116-
}
11171115
return ManagedValue::forLValue(addr);
11181116
}
11191117
}

‎test/Interpreter/moveonly_escaping_definite_initialization.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// RUN: %target-run-simple-swift | %FileCheck %s
22

33
// REQUIRES: executable_test
4-
// REQUIRES: broken
54

65
@_moveOnly
76
struct MO {

‎test/SILGen/moveonly.swift

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ public func consumeVal(_ k: __owned NonTrivialCopyableStruct2) {}
6363
public func consumeVal(_ s: __owned NonTrivialStruct) {}
6464
public func consumeVal(_ s: __owned NonTrivialStruct2) {}
6565

66+
var bool: Bool { false }
67+
6668
///////////
6769
// Tests //
6870
///////////
@@ -803,3 +805,46 @@ struct EmptyStruct {
803805
init() {
804806
}
805807
}
808+
809+
///////////////////////////////
810+
// Conditionally Initialized //
811+
///////////////////////////////
812+
813+
// CHECK: sil hidden [ossa] @$s8moveonly31testConditionallyInitializedLetyyF : $@convention(thin) () -> () {
814+
// CHECK: [[BOX:%.*]] = alloc_box ${ let NonTrivialStruct }, let, name "x"
815+
// CHECK: [[MARK_UNINIT:%.*]] = mark_uninitialized [var] [[BOX]]
816+
// CHECK: [[BORROW:%.*]] = begin_borrow [lexical] [[MARK_UNINIT]]
817+
// CHECK: [[PROJECT:%.*]] = project_box [[BORROW]]
818+
// CHECK: cond_br {{%.*}}, [[LHS_BB:bb[0-9]+]], [[RHS_BB:bb[0-9]+]]
819+
//
820+
// CHECK: [[LHS_BB]]:
821+
// CHECK: [[MARKED:%.*]] = mark_must_check [assignable_but_not_consumable] [[PROJECT]]
822+
// CHECK: assign {{%.*}} to [[MARKED]]
823+
// CHECK: br [[CONT_BB:bb[0-9]+]]
824+
//
825+
// CHECK: [[RHS_BB]]:
826+
// CHECK: [[MARKED:%.*]] = mark_must_check [assignable_but_not_consumable] [[PROJECT]]
827+
// CHECK: assign {{%.*}} to [[MARKED]]
828+
// CHECK: br [[CONT_BB:bb[0-9]+]]
829+
//
830+
// CHECK: [[CONT_BB]]:
831+
// CHECK: [[MARKED:%.*]] = mark_must_check [no_consume_or_assign] [[PROJECT]]
832+
// CHECK: [[BORROW_LOAD:%.*]] = load [copy] [[MARKED]]
833+
// CHECK: apply {{%.*}}([[BORROW_LOAD]])
834+
// CHECK: destroy_value [[BORROW_LOAD]]
835+
// CHECK: [[MARKED:%.*]] = mark_must_check [assignable_but_not_consumable] [[PROJECT]]
836+
// CHECK: [[TAKE_LOAD:%.*]] = load [take] [[MARKED]]
837+
// CHECK: apply {{%.*}}([[TAKE_LOAD]])
838+
// CHECK: } // end sil function '$s8moveonly31testConditionallyInitializedLetyyF'
839+
func testConditionallyInitializedLet() {
840+
let x: NonTrivialStruct
841+
842+
if bool {
843+
x = NonTrivialStruct()
844+
} else {
845+
x = NonTrivialStruct()
846+
}
847+
848+
borrowVal(x)
849+
consumeVal(x)
850+
}

‎test/SILOptimizer/definite_init_moveonly_controlflowdep_init.sil

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
// RUN: %target-sil-opt -module-name definite_init_moveonly_controlflowdep_init -enable-sil-verify-all -definite-init %s | %FileCheck %s
2+
// RUN: %target-sil-opt -module-name definite_init_moveonly_controlflowdep_init -enable-sil-verify-all -definite-init -raw-sil-inst-lowering -sil-move-only-checker -verify %s
3+
4+
// Test that we properly handle both escaping and non-escaping variants.
5+
6+
@_moveOnly
7+
struct S {}
8+
9+
sil @borrowVal : $@convention(thin) (@guaranteed S) -> ()
10+
sil @consumeVal : $@convention(thin) (@owned S) -> ()
11+
sil @getVal : $@convention(thin) () -> @owned S
12+
sil @closureTakeVal : $@convention(thin) (@guaranteed { let S }) -> ()
13+
14+
// CHECK-LABEL: sil hidden [ossa] @letControlFlowDependentInitializationNoEscape : $@convention(thin) () -> () {
15+
// CHECK: [[STACK:%.*]] = alloc_stack
16+
// CHECK: [[MARK:%.*]] = mark_must_check [consumable_and_assignable] [[STACK]]
17+
// CHECK: cond_br undef, [[LHS_BB:bb[0-9]+]], [[RHS_BB:bb[0-9]+]]
18+
//
19+
// CHECK: [[LHS_BB]]:
20+
// CHECK: assign {{%.*}} to [init] [[MARK]]
21+
// CHECK: br [[CONT_BB:bb[0-9]+]]
22+
//
23+
// CHECK: [[RHS_BB]]:
24+
// CHECK: assign {{%.*}} to [init] [[MARK]]
25+
// CHECK: br [[CONT_BB]]
26+
//
27+
// CHECK: [[CONT_BB]]:
28+
// CHECK: [[LOAD:%.*]] = load [copy] [[MARK]]
29+
// CHECK: apply {{%.*}}([[LOAD]])
30+
// CHECK: destroy_value [[LOAD]]
31+
// CHECK: [[LOAD:%.*]] = load [take] [[MARK]]
32+
// CHECK: apply {{%.*}}([[LOAD]])
33+
// CHECK: destroy_addr [[MARK]]
34+
// CHECK: dealloc_stack [[STACK]]
35+
// CHECK: } // end sil function 'letControlFlowDependentInitializationNoEscape'
36+
sil hidden [ossa] @letControlFlowDependentInitializationNoEscape : $@convention(thin) () -> () {
37+
bb0:
38+
%0 = alloc_stack $S, let, name "s"
39+
%1 = mark_uninitialized [var] %0 : $*S
40+
%2 = mark_must_check [consumable_and_assignable] %1 : $*S
41+
cond_br undef, bb1, bb2
42+
43+
bb1:
44+
%8 = function_ref @getVal : $@convention(thin) () -> @owned S
45+
%9 = apply %8() : $@convention(thin) () -> @owned S
46+
assign %9 to %2 : $*S
47+
br bb3
48+
49+
bb2:
50+
%13 = function_ref @getVal : $@convention(thin) () -> @owned S
51+
%14 = apply %13() : $@convention(thin) () -> @owned S
52+
assign %14 to %2 : $*S
53+
br bb3
54+
55+
bb3:
56+
%17 = load [copy] %2 : $*S
57+
%18 = function_ref @borrowVal : $@convention(thin) (@guaranteed S) -> ()
58+
%19 = apply %18(%17) : $@convention(thin) (@guaranteed S) -> ()
59+
destroy_value %17 : $S
60+
%21 = load [take] %2 : $*S
61+
%22 = function_ref @consumeVal : $@convention(thin) (@owned S) -> ()
62+
%23 = apply %22(%21) : $@convention(thin) (@owned S) -> ()
63+
destroy_addr %2 : $*S
64+
dealloc_stack %0 : $*S
65+
%26 = tuple ()
66+
return %26 : $()
67+
}
68+
69+
// CHECK-LABEL: sil hidden [ossa] @letControlFlowDependentInitializationEscape : $@convention(thin) () -> () {
70+
// CHECK: [[BOX:%.*]] = alloc_box
71+
// CHECK: [[PROJECT:%.*]] = project_box [[BOX]]
72+
// CHECK: cond_br undef, [[LHS_BB:bb[0-9]+]], [[RHS_BB:bb[0-9]+]]
73+
//
74+
// CHECK: [[LHS_BB]]:
75+
// CHECK: [[MARK:%.*]] = mark_must_check [initable_but_not_consumable] [[PROJECT]]
76+
// CHECK: assign {{%.*}} to [init] [[MARK]]
77+
// CHECK: br [[CONT_BB:bb[0-9]+]]
78+
//
79+
// CHECK: [[RHS_BB]]:
80+
// CHECK: [[MARK:%.*]] = mark_must_check [initable_but_not_consumable] [[PROJECT]]
81+
// CHECK: assign {{%.*}} to [init] [[MARK]]
82+
// CHECK: br [[CONT_BB]]
83+
//
84+
// CHECK: [[CONT_BB]]:
85+
// CHECK: [[MARK:%.*]] = mark_must_check [no_consume_or_assign] [[PROJECT]]
86+
// CHECK: [[LOAD:%.*]] = load [copy] [[MARK]]
87+
// CHECK: apply {{%.*}}([[LOAD]])
88+
// CHECK: destroy_value [[LOAD]]
89+
//
90+
// The checker will properly error and say that this is illegal on a let.
91+
// CHECK: [[MARK:%.*]] = mark_must_check [assignable_but_not_consumable] [[PROJECT]]
92+
// CHECK: [[LOAD:%.*]] = load [take] [[MARK]]
93+
// CHECK: apply {{%.*}}([[LOAD]])
94+
sil hidden [ossa] @letControlFlowDependentInitializationEscape : $@convention(thin) () -> () {
95+
bb0:
96+
%1 = alloc_box ${ let S }, let, name "s"
97+
%2 = mark_uninitialized [var] %1 : ${ let S }
98+
%3 = project_box %2 : ${ let S }, 0
99+
cond_br undef, bb1, bb2
100+
101+
bb1:
102+
%9 = function_ref @getVal : $@convention(thin) () -> @owned S
103+
%10 = apply %9() : $@convention(thin) () -> @owned S
104+
%11 = mark_must_check [assignable_but_not_consumable] %3 : $*S
105+
assign %10 to %11 : $*S
106+
br bb3
107+
108+
bb2:
109+
%15 = function_ref @getVal : $@convention(thin) () -> @owned S
110+
%16 = apply %15() : $@convention(thin) () -> @owned S
111+
%17 = mark_must_check [assignable_but_not_consumable] %3 : $*S
112+
assign %16 to %17 : $*S
113+
br bb3
114+
115+
bb3:
116+
%20 = mark_must_check [no_consume_or_assign] %3 : $*S
117+
%21 = load [copy] %20 : $*S
118+
%22 = function_ref @borrowVal : $@convention(thin) (@guaranteed S) -> ()
119+
%23 = apply %22(%21) : $@convention(thin) (@guaranteed S) -> ()
120+
destroy_value %21 : $S
121+
%25 = mark_must_check [assignable_but_not_consumable] %3 : $*S
122+
// expected-error @-1 {{'s' was consumed but it is illegal to consume a noncopyable immutable capture of an escaping closure. One can only read from it}}
123+
%26 = load [take] %25 : $*S
124+
%27 = function_ref @consumeVal : $@convention(thin) (@owned S) -> ()
125+
%28 = apply %27(%26) : $@convention(thin) (@owned S) -> ()
126+
%29 = function_ref @closureTakeVal : $@convention(thin) (@guaranteed { let S }) -> ()
127+
%30 = copy_value %2 : ${ let S }
128+
mark_function_escape %3 : $*S
129+
%32 = partial_apply [callee_guaranteed] %29(%30) : $@convention(thin) (@guaranteed { let S }) -> ()
130+
destroy_value %32 : $@callee_guaranteed () -> ()
131+
destroy_value %2 : ${ let S }
132+
%37 = tuple ()
133+
return %37 : $()
134+
}

0 commit comments

Comments
 (0)