Skip to content

Commit 343b8b1

Browse files
kavonkavon
committed
make it illegal to use _forget in an initializer
we decided against allowing it for now. rdar://108877261 (cherry picked from commit 7ac3ea0)
1 parent d7ae2e9 commit 343b8b1

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
@@ -58,28 +58,22 @@ func invokedDeinit() {}
5858
}
5959

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

66+
consuming func tryDestroy(doForget: Bool) throws {
7267
if doForget {
7368
_forget self
7469
}
7570
throw E.err
7671
}
7772

78-
// CHECK-LABEL: sil hidden [ossa] @$s4test11PointerTreeV8doForget4file3ptrACSb_AA4FileVnAA3PtrCtKcfC
73+
// CHECK-LABEL: sil hidden [ossa] @$s4test11PointerTreeV10tryDestroy8doForgetySb_tKF : $@convention(method) (Bool, @owned PointerTree) -> @error any Error {
7974
// CHECK: bb0{{.*}}:
8075
// CHECK: [[SELF_BOX:%.*]] = alloc_box ${ var PointerTree }, var, name "self"
81-
// CHECK: [[MUI:%.*]] = mark_uninitialized [rootself] [[SELF_BOX]] : ${ var PointerTree }
82-
// CHECK: [[SELF:%.*]] = begin_borrow [lexical] [[MUI]] : ${ var PointerTree }
76+
// CHECK: [[SELF:%.*]] = begin_borrow [lexical] [[SELF_BOX]] : ${ var PointerTree }
8377
// CHECK: [[SELF_PTR:%.*]] = project_box [[SELF]] : ${ var PointerTree }, 0
8478
// .. skip to the conditional test ..
8579
// CHECK: [[SHOULD_THROW:%.*]] = struct_extract {{.*}} : $Bool, #Bool._value
@@ -101,13 +95,13 @@ func invokedDeinit() {}
10195
//
10296
// CHECK: bb3:
10397
// CHECK: end_borrow [[SELF]] : ${ var PointerTree }
104-
// CHECK: destroy_value [[MUI]] : ${ var PointerTree }
98+
// CHECK: destroy_value [[SELF_BOX]] : ${ var PointerTree }
10599
// CHECK: throw
106100
// CHECK: } // end sil function
107101

108102
// After the mandatory passes have run, check for correct deinitializations within the init.
109103

110-
// CHECK-SIL-LABEL: sil hidden @$s4test11PointerTreeV8doForget4file3ptrACSb_AA4FileVnAA3PtrCtKcfC
104+
// CHECK-SIL-LABEL: sil hidden @$s4test11PointerTreeV10tryDestroy8doForgetySb_tKF
111105
// CHECK-SIL: [[SHOULD_THROW:%.*]] = struct_extract {{.*}} : $Bool, #Bool._value
112106
// CHECK-SIL: cond_br [[SHOULD_THROW]], bb1, bb2
113107
//
@@ -148,33 +142,31 @@ final class Wallet {
148142
case empty
149143
case within(Wallet)
150144

151-
init(inWallet wallet: Wallet? = nil) {
152-
self = .within(Wallet())
145+
consuming func changeTicket(inWallet wallet: Wallet? = nil) {
153146
if let existingWallet = wallet {
154147
_forget self
155148
self = .within(existingWallet)
156149
}
157150
}
158151
// As of now, we allow reinitialization after forget. Not sure if this is intended.
159-
// CHECK-LABEL: sil hidden [ossa] @$s4test6TicketO8inWalletAcA0D0CSg_tcfC
152+
// CHECK-LABEL: sil hidden [ossa] @$s4test6TicketO06changeB08inWalletyAA0E0CSg_tF : $@convention(method) (@guaranteed Optional<Wallet>, @owned Ticket) -> () {
160153
// CHECK: [[SELF_REF:%.*]] = project_box [[SELF_BOX:%.*]] : ${ var Ticket }, 0
161-
// CHECK: switch_enum {{.*}} : $Optional<Wallet>, case #Optional.some!enumelt: bb2, case #Optional.none!enumelt: bb1
162-
// CHECK: bb2({{%.*}} : @owned $Wallet):
163-
// CHECK: br bb3
154+
// CHECK: switch_enum {{.*}} : $Optional<Wallet>, case #Optional.some!enumelt: [[HAVE_WALLET_BB:bb.*]], case #Optional.none!enumelt: {{.*}}
155+
//
164156
// >> now we begin the destruction sequence, which involves pattern matching on self to destroy its innards
165-
// CHECK: bb3:
157+
// CHECK: [[HAVE_WALLET_BB]]({{%.*}} : @owned $Wallet):
166158
// CHECK: [[SELF_ACCESS:%.*]] = begin_access [read] [unknown] {{%.*}} : $*Ticket
167159
// CHECK: [[SELF_MMC:%.*]] = mark_must_check [no_consume_or_assign] [[SELF_ACCESS]]
168160
// CHECK: [[SELF_COPY:%.*]] = load [copy] [[SELF_MMC]] : $*Ticket
169161
// CHECK: end_access [[SELF_ACCESS:%.*]] : $*Ticket
170-
// CHECK: switch_enum [[SELF_COPY]] : $Ticket, case #Ticket.empty!enumelt: bb4, case #Ticket.within!enumelt: bb5
171-
// CHECK: bb4:
172-
// CHECK: br bb6
173-
// CHECK: bb5([[PREV_SELF_WALLET:%.*]] : @owned $Wallet):
162+
// CHECK: switch_enum [[SELF_COPY]] : $Ticket, case #Ticket.empty!enumelt: [[TICKET_EMPTY:bb[0-9]+]], case #Ticket.within!enumelt: [[TICKET_WITHIN:bb[0-9]+]]
163+
// CHECK: [[TICKET_EMPTY]]:
164+
// CHECK: br [[JOIN_POINT:bb[0-9]+]]
165+
// CHECK: [[TICKET_WITHIN]]([[PREV_SELF_WALLET:%.*]] : @owned $Wallet):
174166
// CHECK: destroy_value [[PREV_SELF_WALLET]] : $Wallet
175-
// CHECK: br bb6
167+
// CHECK: br [[JOIN_POINT]]
176168
// >> from here on we are reinitializing self.
177-
// CHECK: bb6:
169+
// CHECK: [[JOIN_POINT]]:
178170
// CHECK: [[NEW_SELF_VAL:%.*]] = enum $Ticket, #Ticket.within!enumelt, {{.*}} : $Wallet
179171
// CHECK: [[SELF_ACCESS2:%.*]] = begin_access [modify] [unknown] [[SELF_REF]] : $*Ticket
180172
// 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)