Skip to content

Commit ab4da24

Browse files
committed
disallow "non-trivial" stored properties when using forget
We haven't quite got the semantics we want implemented for `discard` aka `_forget` statements. Allowing people to use `_forget` in noncopyable types that have stored properties that require destruction will not let us implement it the way we'd like in the future without source break. But, if the type only has "trivial" stored properties with a no-op destruction, like `Int`, then we can still provide utility for users like FileDescriptor who just want to disable the deinit and have nothing fancy stored in the type itself. rdar://108877261 (cherry picked from commit 578da49)
1 parent fad6484 commit ab4da24

File tree

4 files changed

+154
-22
lines changed

4 files changed

+154
-22
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,18 @@ ERROR(noimplicitcopy_used_on_generic_or_existential, none,
735735
"@_noImplicitCopy can not be used on a generic or existential typed "
736736
"binding or a nominal type containing such typed things", ())
737737

738+
// forget statement
739+
ERROR(forget_nontrivial_storage,none,
740+
"can only 'forget' type %0 if it contains trivially-destroyed "
741+
"stored properties at this time",
742+
(Type))
743+
NOTE(forget_nontrivial_storage_note,none,
744+
"type %0 cannot be trivially destroyed",
745+
(Type))
746+
NOTE(forget_nontrivial_implicit_storage_note,none,
747+
"type %0 implicitly contains %1 which cannot be trivially destroyed",
748+
(Type, Type))
749+
738750
// move only checker diagnostics
739751
ERROR(sil_moveonlychecker_owned_value_consumed_more_than_once, none,
740752
"'%0' consumed more than once", (StringRef))

lib/SILGen/SILGenStmt.cpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -765,6 +765,36 @@ void StmtEmitter::visitForgetStmt(ForgetStmt *S) {
765765
auto *nominal = fn->getDeclContext()->getSelfNominalTypeDecl();
766766
assert(nominal);
767767

768+
// Check if the nominal's contents are trivial. This is a temporary
769+
// restriction until we get forget implemented the way we want.
770+
for (auto *varDecl : nominal->getStoredProperties()) {
771+
assert(varDecl->hasStorage());
772+
auto varType = varDecl->getType();
773+
auto &varTypeLowering = SGF.getTypeLowering(varType);
774+
if (!varTypeLowering.isTrivial()) {
775+
diagnose(getASTContext(),
776+
S->getStartLoc(),
777+
diag::forget_nontrivial_storage,
778+
nominal->getDeclaredInterfaceType());
779+
780+
// emit a note pointing out the problematic storage type
781+
if (auto varLoc = varDecl->getLoc()) {
782+
diagnose(getASTContext(),
783+
varLoc,
784+
diag::forget_nontrivial_storage_note,
785+
varType);
786+
} else {
787+
diagnose(getASTContext(),
788+
nominal->getLoc(),
789+
diag::forget_nontrivial_implicit_storage_note,
790+
nominal->getDeclaredInterfaceType(),
791+
varType);
792+
}
793+
794+
break; // only one diagnostic is needed per forget
795+
}
796+
}
797+
768798
SGF.emitMoveOnlyMemberDestruction(selfValue.forward(SGF), nominal, loc,
769799
nullptr);
770800
}

test/SILGen/forget.swift

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

6262
@_moveOnly struct PointerTree {
63-
let left: Ptr = Ptr()
64-
let file: File = File()
65-
let popularity: Int = 0
66-
var right: Ptr = Ptr()
63+
let left: UnsafePointer<UInt>?
64+
let file: Int = 0
65+
lazy var popularity: Int = 0
66+
var right: Float = 0.0
6767

6868
consuming func tryDestroy(doForget: Bool) throws {
6969
if doForget {
@@ -75,49 +75,39 @@ func invokedDeinit() {}
7575
// CHECK-LABEL: sil hidden [ossa] @$s4test11PointerTreeV10tryDestroy8doForgetySb_tKF : $@convention(method) (Bool, @owned PointerTree) -> @error any Error {
7676
// CHECK: bb0{{.*}}:
7777
// CHECK: [[SELF_BOX:%.*]] = alloc_box ${ var PointerTree }, var, name "self"
78-
// CHECK: [[SELF:%.*]] = begin_borrow [lexical] [[SELF_BOX]] : ${ var PointerTree }
79-
// CHECK: [[SELF_PTR:%.*]] = project_box [[SELF]] : ${ var PointerTree }, 0
78+
// CHECK: [[SELF_PTR:%.*]] = project_box [[SELF_BOX]] : ${ var PointerTree }, 0
8079
// .. skip to the conditional test ..
81-
// CHECK: [[SHOULD_THROW:%.*]] = struct_extract {{.*}} : $Bool, #Bool._value
82-
// CHECK: cond_br [[SHOULD_THROW]], bb1, bb2
80+
// CHECK: [[SHOULD_FORGET:%.*]] = struct_extract {{.*}} : $Bool, #Bool._value
81+
// CHECK: cond_br [[SHOULD_FORGET]], bb1, bb2
8382
//
8483
// CHECK: bb1:
8584
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[SELF_PTR]] : $*PointerTree
8685
// CHECK: [[MMC:%.*]] = mark_must_check [no_consume_or_assign] [[ACCESS]] : $*PointerTree
8786
// CHECK: [[COPIED_SELF:%.*]] = load [copy] [[MMC]] : $*PointerTree
8887
// CHECK: end_access [[ACCESS]] : $*PointerTree
89-
// CHECK: ([[LEFT:%.*]], [[FILE:%.*]], {{%.*}}, [[RIGHT:%.*]]) = destructure_struct [[COPIED_SELF]] : $PointerTree
90-
// CHECK: destroy_value [[LEFT]] : $Ptr
91-
// CHECK: destroy_value [[FILE]] : $File
92-
// CHECK: destroy_value [[RIGHT]] : $Ptr
88+
// CHECK: end_lifetime [[COPIED_SELF]]
9389
// CHECK: br bb3
9490
//
9591
// CHECK: bb2:
9692
// CHECK: br bb3
9793
//
9894
// CHECK: bb3:
99-
// CHECK: end_borrow [[SELF]] : ${ var PointerTree }
10095
// CHECK: destroy_value [[SELF_BOX]] : ${ var PointerTree }
10196
// CHECK: throw
10297
// CHECK: } // end sil function
10398

10499
// After the mandatory passes have run, check for correct deinitializations within the init.
105100

106101
// CHECK-SIL-LABEL: sil hidden @$s4test11PointerTreeV10tryDestroy8doForgetySb_tKF
107-
// CHECK-SIL: [[SHOULD_THROW:%.*]] = struct_extract {{.*}} : $Bool, #Bool._value
108-
// CHECK-SIL: cond_br [[SHOULD_THROW]], bb1, bb2
102+
// CHECK-SIL: [[SHOULD_FORGET:%.*]] = struct_extract {{.*}} : $Bool, #Bool._value
103+
// CHECK-SIL: cond_br [[SHOULD_FORGET]], bb1, bb2
109104
//
110105
// CHECK-SIL: bb1:
111106
// CHECK-SIL: [[ACCESS:%.*]] = begin_access [modify] [static] {{.*}} : $*PointerTree
112107
// CHECK-SIL: [[SELF_VAL:%.*]] = load [[ACCESS]] : $*PointerTree
113108
// CHECK-SIL: end_access [[ACCESS]] : $*PointerTree
114-
// CHECK-SIL: [[LEFT:%.*]] = struct_extract [[SELF_VAL]] : $PointerTree, #PointerTree.left
115-
// CHECK-SIL: [[FILE:%.*]] = struct_extract [[SELF_VAL]] : $PointerTree, #PointerTree.file
116-
// CHECK-SIL: [[RIGHT:%.*]] = struct_extract [[SELF_VAL]] : $PointerTree, #PointerTree.right
117-
// CHECK-SIL: strong_release [[LEFT]] : $Ptr
118-
// CHECK-SIL: [[FILE_DEINIT:%.*]] = function_ref @$s4test4FileVfD : $@convention(method) (@owned File) -> ()
119-
// CHECK-SIL: apply [[FILE_DEINIT]]([[FILE]])
120-
// CHECK-SIL: strong_release [[RIGHT]] : $Ptr
109+
// CHECK-SIL-NOT: struct_extract
110+
// no accesses to the fields are expected because the fields are no-op destroyed.
121111
// CHECK-SIL: br bb3
122112
//
123113
// CHECK-SIL: bb2:
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
// RUN: %target-swift-emit-silgen -verify %s
2+
3+
// Yes, you read that right. This is being checked in SILGen! So we have to
4+
// separate these tests from ones emitting errors during Sema.
5+
6+
class ClassyGal {}
7+
8+
@propertyWrapper
9+
struct Appending<Val> {
10+
let terminator: Val
11+
var store: [Val] = []
12+
13+
var wrappedValue: [Val] {
14+
get {
15+
return store + [terminator]
16+
}
17+
set {
18+
fatalError("mutation is the root of all bugs")
19+
}
20+
}
21+
}
22+
23+
struct StructuredGuy: ~Copyable {
24+
let x = ClassyGal()
25+
// expected-note@-1 {{type 'ClassyGal' cannot be trivially destroyed}}
26+
27+
@Appending(terminator: ClassyGal())
28+
var bestC: [ClassyGal]
29+
30+
consuming func doForget() { _forget self }
31+
// expected-error@-1 {{can only 'forget' type 'StructuredGuy' if it contains trivially-destroyed stored properties at this time}}
32+
deinit {}
33+
}
34+
35+
struct AppendyEnby: ~Copyable {
36+
@Appending(terminator: 0)
37+
var string: [Int]
38+
// expected-note@-1 {{type 'Appending<Int>' cannot be trivially destroyed}}
39+
40+
41+
consuming func doForget() { _forget self }
42+
// expected-error@-1 {{can only 'forget' type 'AppendyEnby' if it contains trivially-destroyed stored properties at this time}}
43+
deinit {}
44+
}
45+
46+
struct HasGeneric: ~Copyable {
47+
var thing: Any // expected-note {{type 'Any' cannot be trivially destroyed}}
48+
49+
consuming func forget() { _forget self }
50+
// expected-error@-1 {{can only 'forget' type 'HasGeneric' if it contains trivially-destroyed stored properties at this time}}
51+
52+
deinit{}
53+
}
54+
55+
struct WrappingNoncopyable: ~Copyable {
56+
var computed: String { "mine" }
57+
58+
var x: AppendyEnby // expected-note{{type 'AppendyEnby' cannot be trivially destroyed}}
59+
consuming func doForget() { _forget self }
60+
// expected-error@-1 {{can only 'forget' type 'WrappingNoncopyable' if it contains trivially-destroyed stored properties at this time}}
61+
deinit {}
62+
}
63+
64+
struct LazyGuy: ~Copyable {
65+
lazy var thing: String = "asdf"
66+
// expected-note@-1 {{type 'String?' cannot be trivially destroyed}}
67+
68+
consuming func doForget() { _forget self }
69+
// expected-error@-1 {{can only 'forget' type 'LazyGuy' if it contains trivially-destroyed stored properties at this time}}
70+
deinit {}
71+
}
72+
73+
struct BoringNoncopyable: ~Copyable {
74+
let x = 0
75+
let y = 0
76+
}
77+
78+
struct Boring {
79+
var x = 0
80+
var y = 1.9
81+
}
82+
83+
// FIXME: Despite not having a deinit, the noncopyable struct isn't considered trivial?
84+
struct ContainsBoring: ~Copyable {
85+
let z: BoringNoncopyable // expected-note {{type 'BoringNoncopyable' cannot be trivially destroyed}}
86+
consuming func forget() { _forget self }
87+
// expected-error@-1 {{can only 'forget' type 'ContainsBoring' if it contains trivially-destroyed stored properties at this time}}
88+
deinit {}
89+
}
90+
91+
struct AllOK: ~Copyable {
92+
var maybeDigits: Int?
93+
var maybeFloat: Float?
94+
let dbl: Double
95+
var location: Boring = Boring()
96+
var unsafePtr: UnsafePointer<Int>
97+
98+
consuming func doForget() { _forget self }
99+
deinit {}
100+
}

0 commit comments

Comments
 (0)