Skip to content

Commit c82ce37

Browse files
committed
Fix MoveOnlyAddressChecker to handle value deinits.
Track liveness of self so we don't accidentally think that such types can be memberwise reinitialized.
1 parent 2a78957 commit c82ce37

File tree

4 files changed

+83
-1
lines changed

4 files changed

+83
-1
lines changed

lib/SIL/Utils/FieldSensitivePrunedLiveness.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,13 @@ TypeSubElementCount::TypeSubElementCount(SILType type, SILModule &mod,
7070
type.getFieldType(fieldDecl, mod, context), mod, context);
7171
number = numElements;
7272

73+
if (type.isValueTypeWithDeinit()) {
74+
// 'self' has its own liveness represented as an additional field at the
75+
// end of the structure.
76+
++number;
77+
}
7378
// If we do not have any elements, just set our size to 1.
74-
if (numElements == 0)
79+
if (number == 0)
7580
number = 1;
7681

7782
return;
@@ -350,6 +355,10 @@ void TypeTreeLeafTypeRange::constructFilteredProjections(
350355
callback(newValue, TypeTreeLeafTypeRange(start, next));
351356
start = next;
352357
}
358+
if (type.isValueTypeWithDeinit()) {
359+
// 'self' has its own liveness
360+
++start;
361+
}
353362
assert(start == endEltOffset);
354363
return;
355364
}

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,7 @@ static bool memInstMustConsume(Operand *memOper) {
437437

438438
SILInstruction *memInst = memOper->getUser();
439439

440+
// FIXME: drop_deinit must be handled here!
440441
switch (memInst->getKind()) {
441442
default:
442443
return false;

test/Interpreter/moveonly_discard.swift

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,34 @@ struct SillyEmptyGeneric<T>: ~Copyable {
9191
deinit { fatalError("ran unexpectedly!") }
9292
}
9393

94+
struct SingleMutableField: ~Copyable {
95+
var value = 0
96+
97+
consuming func justDiscard() {
98+
discard self
99+
}
100+
101+
deinit {
102+
print("SingleMutableField.deinit")
103+
}
104+
}
105+
106+
// rdar://110232973 ([move-only] Checker should distinguish in between
107+
// field of single field struct vs parent field itself (was: mutation
108+
// of field in noncopyable struct should not trigger deinit))
109+
//
110+
// This test must not be in a closure.
111+
@inline(never)
112+
func testSingleMutableFieldNoMemberReinit() {
113+
var x = SingleMutableField()
114+
x.value = 20 // should not trigger deinit.
115+
// CHECK-NOT: SingleMutableField.deinit
116+
x.justDiscard()
117+
}
118+
94119
func main() {
120+
testSingleMutableFieldNoMemberReinit()
121+
95122
let _ = {
96123
let x = FileDescriptor() // 0
97124
x.close()

test/SILOptimizer/moveonly_addresschecker.sil

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
sil_stage raw
44

55
import Swift
6+
import Builtin
67

78
public class CopyableKlass {}
89

@@ -52,6 +53,17 @@ final class ClassContainingMoveOnly {
5253
var value = NonTrivialStruct()
5354
}
5455

56+
struct SingleTrivialFieldAndDeinit: ~Copyable {
57+
var value: Builtin.IntLiteral
58+
59+
consuming func finalize()
60+
61+
deinit
62+
}
63+
64+
sil @getSingleTrivialFieldAndDeinit : $@convention(thin) () -> @owned SingleTrivialFieldAndDeinit
65+
sil @finalizeSingleTrivialFieldAndDeinit : $@convention(thin) (@owned SingleTrivialFieldAndDeinit) -> ()
66+
5567
@_hasStorage @_hasInitialValue var varGlobal: NonTrivialStruct { get set }
5668
@_hasStorage @_hasInitialValue let letGlobal: NonTrivialStruct { get }
5769
sil_global hidden @$s23moveonly_addresschecker9varGlobalAA16NonTrivialStructVvp : $NonTrivialStruct
@@ -632,3 +644,36 @@ bb0:
632644
%22 = tuple ()
633645
return %22 : $()
634646
}
647+
648+
// rdar://110232973 ([move-only] Checker should distinguish in between field of single field struct
649+
// vs parent field itself (was: mutation of field in noncopyable struct should not trigger deinit))
650+
//
651+
// Test that the SingleTrivialFieldAndDeinit aggregate is not
652+
// deinitialized when it's only field is consumed.
653+
//
654+
// CHECK-LABEL: sil hidden [ossa] @testNoFieldReinit : $@convention(thin) () -> () {
655+
// CHECK: [[ALLOC:%.*]] = alloc_stack [lexical] $SingleTrivialFieldAndDeinit
656+
// CHECK-NOT: destroy_addr [[ALLOC]]
657+
// CHECK-LABEL: } // end sil function 'testNoFieldReinit'
658+
sil hidden [ossa] @testNoFieldReinit : $@convention(thin) () -> () {
659+
bb0:
660+
%0 = alloc_stack [lexical] $SingleTrivialFieldAndDeinit, var
661+
%1 = mark_must_check [consumable_and_assignable] %0 : $*SingleTrivialFieldAndDeinit
662+
%3 = function_ref @getSingleTrivialFieldAndDeinit : $@convention(thin) () -> @owned SingleTrivialFieldAndDeinit
663+
%4 = apply %3() : $@convention(thin) () -> @owned SingleTrivialFieldAndDeinit
664+
store %4 to [init] %1 : $*SingleTrivialFieldAndDeinit
665+
%6 = integer_literal $Builtin.IntLiteral, 20
666+
%10 = begin_access [modify] [static] %1 : $*SingleTrivialFieldAndDeinit
667+
%11 = struct_element_addr %10 : $*SingleTrivialFieldAndDeinit, #SingleTrivialFieldAndDeinit.value
668+
store %6 to [trivial] %11 : $*Builtin.IntLiteral
669+
end_access %10 : $*SingleTrivialFieldAndDeinit
670+
%14 = begin_access [deinit] [static] %1 : $*SingleTrivialFieldAndDeinit
671+
%15 = load [take] %14 : $*SingleTrivialFieldAndDeinit
672+
%16 = function_ref @finalizeSingleTrivialFieldAndDeinit : $@convention(thin) (@owned SingleTrivialFieldAndDeinit) -> ()
673+
%17 = apply %16(%15) : $@convention(thin) (@owned SingleTrivialFieldAndDeinit) -> ()
674+
end_access %14 : $*SingleTrivialFieldAndDeinit
675+
destroy_addr %1 : $*SingleTrivialFieldAndDeinit
676+
dealloc_stack %0 : $*SingleTrivialFieldAndDeinit
677+
%21 = tuple ()
678+
return %21 : $()
679+
}

0 commit comments

Comments
 (0)