Skip to content

Commit b61a73d

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. Fixes rdar://110232973 ([move-only] Checker should distinguish in between field of single field struct vs parent field itself (was: mutation of field in noncopyable struct should not trigger deinit)) (cherry picked from commit a8c45c5)
1 parent 4eb81e7 commit b61a73d

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
@@ -90,7 +90,34 @@ struct SillyEmptyGeneric<T>: ~Copyable {
9090
deinit { fatalError("ran unexpectedly!") }
9191
}
9292

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