Skip to content

Commit 7ac3ea0

Browse files
kavonkavon
committed
make it illegal to use _forget in an initializer
we decided against allowing it for now. rdar://108877261
1 parent 3d7df83 commit 7ac3ea0

File tree

5 files changed

+38
-48
lines changed

5 files changed

+38
-48
lines changed

‎lib/Sema/TypeCheckStmt.cpp

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1251,7 +1251,8 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
12511251
// Save this for SILGen, since Stmt's don't know their decl context.
12521252
FS->setInnermostMethodContext(fn);
12531253

1254-
if (fn->isStatic() || isa<DestructorDecl>(fn)) {
1254+
if (fn->isStatic() || isa<DestructorDecl>(fn)
1255+
|| isa<ConstructorDecl>(fn)) {
12551256
ctx.Diags.diagnose(FS->getForgetLoc(), diag::forget_wrong_context_decl,
12561257
fn->getDescriptiveKind());
12571258
diagnosed = true;
@@ -1321,30 +1322,21 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
13211322

13221323
// The 'self' parameter must be owned (aka "consuming").
13231324
if (!diagnosed) {
1324-
bool isConsuming = false;
13251325
if (auto *funcDecl = dyn_cast<FuncDecl>(fn)) {
13261326
switch (funcDecl->getSelfAccessKind()) {
13271327
case SelfAccessKind::LegacyConsuming:
13281328
case SelfAccessKind::Consuming:
1329-
isConsuming = true;
13301329
break;
13311330

13321331
case SelfAccessKind::Borrowing:
13331332
case SelfAccessKind::NonMutating:
13341333
case SelfAccessKind::Mutating:
1335-
isConsuming = false;
1334+
ctx.Diags.diagnose(FS->getForgetLoc(),
1335+
diag::forget_wrong_context_nonconsuming,
1336+
fn->getDescriptiveKind());
1337+
diagnosed = true;
13361338
break;
13371339
}
1338-
} else if (isa<ConstructorDecl>(fn)) {
1339-
// constructors are implicitly "consuming" of the self instance.
1340-
isConsuming = true;
1341-
}
1342-
1343-
if (!isConsuming) {
1344-
ctx.Diags.diagnose(FS->getForgetLoc(),
1345-
diag::forget_wrong_context_nonconsuming,
1346-
fn->getDescriptiveKind());
1347-
diagnosed = true;
13481340
}
13491341
}
13501342

‎test/Interpreter/moveonly_forget.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ struct FileDescriptor {
1414
var fd: Int
1515
static var nextFD: Int = 0
1616

17+
consuming func forget() { _forget self }
18+
1719
init() {
1820
self.fd = FileDescriptor.nextFD
1921
FileDescriptor.nextFD += 1
@@ -22,7 +24,7 @@ struct FileDescriptor {
2224
init(doForget: Bool) throws {
2325
self.init()
2426
if doForget {
25-
_forget self
27+
forget()
2628
throw E.err
2729
}
2830
}
@@ -61,10 +63,12 @@ struct FileDescriptor {
6163
case some(FileDescriptor)
6264
case nothing
6365

66+
consuming func forget() { _forget self }
67+
6468
init(reinit: Bool) {
6569
self = .some(FileDescriptor())
6670
if reinit {
67-
_forget self
71+
forget()
6872
self = .some(FileDescriptor())
6973
}
7074
}

‎test/SILGen/forget.swift

Lines changed: 20 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -60,28 +60,22 @@ func invokedDeinit() {}
6060
}
6161

6262
@_moveOnly struct PointerTree {
63-
let left: Ptr
64-
let file: File
65-
let popularity: Int
66-
var right: Ptr
67-
68-
init(doForget: Bool, file: __owned File, ptr: Ptr) throws {
69-
self.file = file
70-
self.left = ptr
71-
self.popularity = 0
72-
self.right = ptr
63+
let left: Ptr = Ptr()
64+
let file: File = File()
65+
let popularity: Int = 0
66+
var right: Ptr = Ptr()
7367

68+
consuming func tryDestroy(doForget: Bool) throws {
7469
if doForget {
7570
_forget self
7671
}
7772
throw E.err
7873
}
7974

80-
// CHECK-LABEL: sil hidden [ossa] @$s4test11PointerTreeV8doForget4file3ptrACSb_AA4FileVnAA3PtrCtKcfC
75+
// CHECK-LABEL: sil hidden [ossa] @$s4test11PointerTreeV10tryDestroy8doForgetySb_tKF : $@convention(method) (Bool, @owned PointerTree) -> @error any Error {
8176
// CHECK: bb0{{.*}}:
8277
// CHECK: [[SELF_BOX:%.*]] = alloc_box ${ var PointerTree }, var, name "self"
83-
// CHECK: [[MUI:%.*]] = mark_uninitialized [rootself] [[SELF_BOX]] : ${ var PointerTree }
84-
// CHECK: [[SELF:%.*]] = begin_borrow [lexical] [[MUI]] : ${ var PointerTree }
78+
// CHECK: [[SELF:%.*]] = begin_borrow [lexical] [[SELF_BOX]] : ${ var PointerTree }
8579
// CHECK: [[SELF_PTR:%.*]] = project_box [[SELF]] : ${ var PointerTree }, 0
8680
// .. skip to the conditional test ..
8781
// CHECK: [[SHOULD_THROW:%.*]] = struct_extract {{.*}} : $Bool, #Bool._value
@@ -104,13 +98,13 @@ func invokedDeinit() {}
10498
//
10599
// CHECK: bb3:
106100
// CHECK: end_borrow [[SELF]] : ${ var PointerTree }
107-
// CHECK: destroy_value [[MUI]] : ${ var PointerTree }
101+
// CHECK: destroy_value [[SELF_BOX]] : ${ var PointerTree }
108102
// CHECK: throw
109103
// CHECK: } // end sil function
110104

111105
// After the mandatory passes have run, check for correct deinitializations within the init.
112106

113-
// CHECK-SIL-LABEL: sil hidden @$s4test11PointerTreeV8doForget4file3ptrACSb_AA4FileVnAA3PtrCtKcfC
107+
// CHECK-SIL-LABEL: sil hidden @$s4test11PointerTreeV10tryDestroy8doForgetySb_tKF
114108
// CHECK-SIL: [[SHOULD_THROW:%.*]] = struct_extract {{.*}} : $Bool, #Bool._value
115109
// CHECK-SIL: cond_br [[SHOULD_THROW]], bb1, bb2
116110
//
@@ -151,34 +145,32 @@ final class Wallet {
151145
case empty
152146
case within(Wallet)
153147

154-
init(inWallet wallet: Wallet? = nil) {
155-
self = .within(Wallet())
148+
consuming func changeTicket(inWallet wallet: Wallet? = nil) {
156149
if let existingWallet = wallet {
157150
_forget self
158151
self = .within(existingWallet)
159152
}
160153
}
161154
// As of now, we allow reinitialization after forget. Not sure if this is intended.
162-
// CHECK-LABEL: sil hidden [ossa] @$s4test6TicketO8inWalletAcA0D0CSg_tcfC
155+
// CHECK-LABEL: sil hidden [ossa] @$s4test6TicketO06changeB08inWalletyAA0E0CSg_tF : $@convention(method) (@guaranteed Optional<Wallet>, @owned Ticket) -> () {
163156
// CHECK: [[SELF_REF:%.*]] = project_box [[SELF_BOX:%.*]] : ${ var Ticket }, 0
164-
// CHECK: switch_enum {{.*}} : $Optional<Wallet>, case #Optional.some!enumelt: bb2, case #Optional.none!enumelt: bb1
165-
// CHECK: bb2({{%.*}} : @owned $Wallet):
166-
// CHECK: br bb3
157+
// CHECK: switch_enum {{.*}} : $Optional<Wallet>, case #Optional.some!enumelt: [[HAVE_WALLET_BB:bb.*]], case #Optional.none!enumelt: {{.*}}
158+
//
167159
// >> now we begin the destruction sequence, which involves pattern matching on self to destroy its innards
168-
// CHECK: bb3:
160+
// CHECK: [[HAVE_WALLET_BB]]({{%.*}} : @owned $Wallet):
169161
// CHECK: [[SELF_ACCESS:%.*]] = begin_access [read] [unknown] {{%.*}} : $*Ticket
170162
// CHECK: [[SELF_MMC:%.*]] = mark_must_check [no_consume_or_assign] [[SELF_ACCESS]]
171163
// CHECK: [[SELF_COPY:%.*]] = load [copy] [[SELF_MMC]] : $*Ticket
172164
// CHECK: end_access [[SELF_ACCESS:%.*]] : $*Ticket
173165
// CHECK: [[DD:%.*]] = drop_deinit [[SELF_COPY]] : $Ticket
174-
// CHECK: switch_enum [[DD]] : $Ticket, case #Ticket.empty!enumelt: bb4, case #Ticket.within!enumelt: bb5
175-
// CHECK: bb4:
176-
// CHECK: br bb6
177-
// CHECK: bb5([[PREV_SELF_WALLET:%.*]] : @owned $Wallet):
166+
// CHECK: switch_enum [[DD]] : $Ticket, case #Ticket.empty!enumelt: [[TICKET_EMPTY:bb[0-9]+]], case #Ticket.within!enumelt: [[TICKET_WITHIN:bb[0-9]+]]
167+
// CHECK: [[TICKET_EMPTY]]:
168+
// CHECK: br [[JOIN_POINT:bb[0-9]+]]
169+
// CHECK: [[TICKET_WITHIN]]([[PREV_SELF_WALLET:%.*]] : @owned $Wallet):
178170
// CHECK: destroy_value [[PREV_SELF_WALLET]] : $Wallet
179-
// CHECK: br bb6
171+
// CHECK: br [[JOIN_POINT]]
180172
// >> from here on we are reinitializing self.
181-
// CHECK: bb6:
173+
// CHECK: [[JOIN_POINT]]:
182174
// CHECK: [[NEW_SELF_VAL:%.*]] = enum $Ticket, #Ticket.within!enumelt, {{.*}} : $Wallet
183175
// CHECK: [[SELF_ACCESS2:%.*]] = begin_access [modify] [unknown] [[SELF_REF]] : $*Ticket
184176
// CHECK: [[SELF_MMC2:%.*]] = mark_must_check [assignable_but_not_consumable] [[SELF_ACCESS2]] : $*Ticket

‎test/SILOptimizer/moveonly_forget.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,12 @@ final class Wallet {
6464
case pagedOut(GoodFileDescriptor)
6565
case within(Wallet)
6666

67+
consuming func forget() { _forget self }
68+
6769
init(inWallet wallet: Wallet? = nil) { // expected-error {{'self' consumed more than once}}
6870
self = .within(Wallet())
6971
if let existingWallet = wallet {
70-
_forget self
72+
forget()
7173
self = .within(existingWallet)
7274
}
7375
forget(forever: true) // expected-note {{consuming use here}}

‎test/Sema/forget.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,15 @@ enum E: Error { case err }
4545
init() throws {
4646
self.fd = 0
4747

48-
_forget self
48+
_forget self // expected-error {{'forget' statement cannot appear in initializer}}
4949

5050
throw E.err
5151
}
5252

5353
init(_ b: Bool) throws {
5454
try self.init()
5555

56-
_forget self
56+
_forget self // expected-error {{'forget' statement cannot appear in initializer}}
5757

5858
throw E.err
5959
}
@@ -122,7 +122,7 @@ enum E: Error { case err }
122122
case nothing
123123

124124
init() throws {
125-
_forget self
125+
_forget self // expected-error {{'forget' statement cannot appear in initializer}}
126126
throw E.err
127127
}
128128

0 commit comments

Comments
 (0)