Skip to content

Commit fce0d33

Browse files
xedinxedin
authored
Merge pull request #68155 from xedin/issue-67827
[SILGen] InitAccessors: Make sure that `assign_or_init` always directly references self
2 parents 537770c + 10947de commit fce0d33

File tree

4 files changed

+127
-10
lines changed

4 files changed

+127
-10
lines changed

‎lib/SILGen/SILGenFunction.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1893,7 +1893,20 @@ void SILGenFunction::emitAssignOrInit(SILLocation loc, ManagedValue selfValue,
18931893
setterFRef = SILUndef::get(initFRef->getType(), F);
18941894
}
18951895

1896-
B.createAssignOrInit(loc, field, selfValue.getValue(),
1896+
auto isValueSelf = !selfValue.getType().getASTType()->mayHaveSuperclass();
1897+
// If we are emitting `assign_or_init` instruction for a value
1898+
// type, we need to make sure that "self" is always a l-value
1899+
// reference to "rootself" because `nonmutating set` loads "self"
1900+
// and referencing `selfValue` in such case is incorrect because
1901+
// it's a copy which is going to be skipped by DI.
1902+
auto selfRef = selfValue;
1903+
if (isValueSelf && !selfRef.isLValue()) {
1904+
auto *ctor = cast<ConstructorDecl>(FunctionDC->getAsDecl());
1905+
selfRef = maybeEmitValueOfLocalVarDecl(ctor->getImplicitSelfDecl(),
1906+
AccessKind::ReadWrite);
1907+
}
1908+
1909+
B.createAssignOrInit(loc, field, selfRef.getValue(),
18971910
newValue.forward(*this), initFRef, setterFRef,
18981911
AssignOrInitInst::Unknown);
18991912
}

‎lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -658,16 +658,16 @@ bool LifetimeChecker::shouldEmitError(const SILInstruction *Inst) {
658658
}))
659659
return false;
660660

661-
// Ignore loads used only by an assign_by_wrapper setter. This
662-
// is safe to ignore because assign_by_wrapper will only be
663-
// re-written to use the setter if the value is fully initialized.
661+
// Ignore loads used only by an assign_by_wrapper or assign_or_init setter.
662+
// This is safe to ignore because assign_by_wrapper/assign_or_init will
663+
// only be re-written to use the setter if the value is fully initialized.
664664
if (auto *load = dyn_cast<SingleValueInstruction>(Inst)) {
665665
if (auto Op = load->getSingleUse()) {
666666
if (auto PAI = dyn_cast<PartialApplyInst>(Op->getUser())) {
667-
if (std::find_if(PAI->use_begin(), PAI->use_end(),
668-
[](auto PAIUse) {
669-
return isa<AssignByWrapperInst>(PAIUse->getUser());
670-
}) != PAI->use_end()) {
667+
if (std::find_if(PAI->use_begin(), PAI->use_end(), [](auto PAIUse) {
668+
return isa<AssignByWrapperInst>(PAIUse->getUser()) ||
669+
isa<AssignOrInitInst>(PAIUse->getUser());
670+
}) != PAI->use_end()) {
671671
return false;
672672
}
673673
}

‎test/Interpreter/init_accessors.swift

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -847,3 +847,77 @@ do {
847847
// CHECK-NEXT: TestMixedDefaultInitalizable(a: nil, b: Optional(""))
848848
// CHECK-NEXT: TestMixedDefaultInitalizable(a: nil, b: Optional("Hello"))
849849
// CHECK-NEXT: TestMixedDefaultInitalizable(a: nil, b: Optional(""))
850+
851+
do {
852+
struct TestNonMutatingSetDefault {
853+
var _count: Int
854+
855+
var count: Int = 42 {
856+
@storageRestrictions(initializes: _count)
857+
init {
858+
_count = newValue
859+
}
860+
861+
get { _count }
862+
nonmutating set {}
863+
}
864+
}
865+
866+
struct TestNonMutatingSetNoDefault {
867+
var _count: Int
868+
869+
var count: Int {
870+
@storageRestrictions(initializes: _count)
871+
init {
872+
print("init accessor is called: \(newValue)")
873+
_count = newValue
874+
}
875+
876+
get { _count }
877+
878+
nonmutating set {
879+
print("nonmutating set called: \(newValue)")
880+
}
881+
}
882+
883+
init(value: Int) {
884+
self.count = value
885+
self.count = value + 1
886+
}
887+
}
888+
889+
struct TestNonMutatingSetCustom {
890+
var _count: Int
891+
892+
var count: Int = 42 {
893+
@storageRestrictions(initializes: _count)
894+
init {
895+
print("init accessor is called: \(newValue)")
896+
_count = newValue
897+
}
898+
899+
get { _count }
900+
901+
nonmutating set {
902+
print("nonmutating set called: \(newValue)")
903+
}
904+
}
905+
906+
init(custom: Int) {
907+
count = custom
908+
}
909+
}
910+
911+
print("test-nonmutating-set-1: \(TestNonMutatingSetDefault())")
912+
print("test-nonmutating-set-2: \(TestNonMutatingSetDefault(count: 0))")
913+
print("test-nonmutating-set-3: \(TestNonMutatingSetNoDefault(value: -1))")
914+
print("test-nonmutating-set-4: \(TestNonMutatingSetCustom(custom: 0))")
915+
}
916+
// CHECK: test-nonmutating-set-1: TestNonMutatingSetDefault(_count: 42)
917+
// CHECK-NEXT: test-nonmutating-set-2: TestNonMutatingSetDefault(_count: 0)
918+
// CHECK-NEXT: init accessor is called: -1
919+
// CHECK-NEXT: nonmutating set called: 0
920+
// CHECK-NEXT: test-nonmutating-set-3: TestNonMutatingSetNoDefault(_count: -1)
921+
// CHECK-NEXT: init accessor is called: 42
922+
// CHECK-NEXT: nonmutating set called: 0
923+
// CHECK-NEXT: test-nonmutating-set-4: TestNonMutatingSetCustom(_count: 42)

‎test/SILOptimizer/init_accessor_raw_sil_lowering.swift

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ func test_default_inits() {
225225
// CHECK: [[INIT_ACCESSOR:%.*]] = function_ref @$s23assign_or_init_lowering18test_default_initsyyF5Test1L_V1xSivi : $@convention(thin) (Int) -> @out Int
226226
// CHECK: [[SETTER_REF:%.*]] = function_ref @$s23assign_or_init_lowering18test_default_initsyyF5Test1L_V1xSivs : $@convention(method) (Int, @inout Test1) -> ()
227227
// CHECK-NEXT: [[SETTER:%.*]] = partial_apply [callee_guaranteed] [[SETTER_REF]]([[SELF_REF]]) : $@convention(method) (Int, @inout Test1) -> ()
228-
// CHECK-NEXT: assign_or_init [init] #<abstract function>Test1.x, self [[SELF_REF]] : $*Test1, value [[INIT_VAL]] : $Int, init [[INIT_ACCESSOR]] : $@convention(thin) (Int) -> @out Int, set [[SETTER]] : $@callee_guaranteed (Int) -> ()
228+
// CHECK-NEXT: assign_or_init [init] #<abstract function>Test1.x, self %1 : $*Test1, value [[INIT_VAL]] : $Int, init [[INIT_ACCESSOR]] : $@convention(thin) (Int) -> @out Int, set [[SETTER]] : $@callee_guaranteed (Int) -> ()
229229
// CHECK-NEXT: end_access [[SELF_REF]] : $*Test1
230230
// CHECK-NEXT: destroy_value [[SETTER]] : $@callee_guaranteed (Int) -> ()
231231
init() {
@@ -238,7 +238,7 @@ func test_default_inits() {
238238
// CHECK: [[INIT_ACCESSOR:%.*]] = function_ref @$s23assign_or_init_lowering18test_default_initsyyF5Test1L_V1xSivi : $@convention(thin) (Int) -> @out Int
239239
// CHECK: [[SETTER_REF:%.*]] = function_ref @$s23assign_or_init_lowering18test_default_initsyyF5Test1L_V1xSivs : $@convention(method) (Int, @inout Test1) -> ()
240240
// CHECK-NEXT: [[SETTER:%.*]] = partial_apply [callee_guaranteed] [[SETTER_REF]]([[SELF_REF]]) : $@convention(method) (Int, @inout Test1) -> ()
241-
// CHECK-NEXT: assign_or_init [init] #<abstract function>Test1.x, self [[SELF_REF]] : $*Test1, value [[INIT_VAL]] : $Int, init [[INIT_ACCESSOR]] : $@convention(thin) (Int) -> @out Int, set [[SETTER]] : $@callee_guaranteed (Int) -> ()
241+
// CHECK-NEXT: assign_or_init [init] #<abstract function>Test1.x, self %2 : $*Test1, value [[INIT_VAL]] : $Int, init [[INIT_ACCESSOR]] : $@convention(thin) (Int) -> @out Int, set [[SETTER]] : $@callee_guaranteed (Int) -> ()
242242
// CHECK-NEXT: end_access [[SELF_REF]] : $*Test1
243243
// CHECK-NEXT: destroy_value [[SETTER]] : $@callee_guaranteed (Int) -> ()
244244
//
@@ -396,3 +396,33 @@ func test_handling_of_superclass_properties() {
396396
}
397397
}
398398
}
399+
400+
func test_handling_of_nonmutating_set() {
401+
struct Test {
402+
private var _count: Int
403+
404+
var count: Int = 42 {
405+
@storageRestrictions(initializes: _count)
406+
init {
407+
_count = newValue
408+
}
409+
get {
410+
_count
411+
}
412+
nonmutating set {
413+
// Update store
414+
}
415+
}
416+
417+
// CHECK-LABEL: sil private [ossa] @$s23assign_or_init_lowering32test_handling_of_nonmutating_setyyF4TestL_V5countADSi_tcfC : $@convention(method) (Int, @thin Test.Type) -> Test
418+
// CHECK: [[INIT_VALUE:%.*]] = function_ref @$s23assign_or_init_lowering32test_handling_of_nonmutating_setyyF4TestL_V5countSivpfi : $@convention(thin) () -> Int
419+
// CHECK-NEXT: [[VALUE:%.*]] = apply [[INIT_VALUE]]() : $@convention(thin) () -> Int
420+
// CHECK: assign_or_init [init] #<abstract function>Test.count, self %2 : $*Test, value [[VALUE]] : $Int, init {{.*}} : $@convention(thin) (Int) -> @out Int, set {{.*}} : $@callee_guaranteed (Int) -> ()
421+
// CHECK: assign_or_init [set] #<abstract function>Test.count, self %2 : $*Test, value %0 : $Int, init {{.*}} : $@convention(thin) (Int) -> @out Int, set {{.*}} : $@callee_guaranteed (Int) -> ()
422+
// CHECK: assign_or_init [set] #<abstract function>Test.count, self %2 : $*Test, value [[ZERO:%.*]] : $Int, init {{.*}} : $@convention(thin) (Int) -> @out Int, set {{.*}} : $@callee_guaranteed (Int) -> ()
423+
init(count: Int) {
424+
self.count = count
425+
self.count = 0
426+
}
427+
}
428+
}

0 commit comments

Comments
 (0)