Skip to content

Commit c9e8cd0

Browse files
xedinxedin
authored
Merge pull request #68051 from xedin/init-accessors-default-init-improvements-5.9
[5.9][TypeChecker/DI] InitAccessors: Treat properties without "initializes" as stored fix default initialization handling
2 parents 33929b3 + 9656f26 commit c9e8cd0

File tree

11 files changed

+328
-18
lines changed

11 files changed

+328
-18
lines changed

‎include/swift/SIL/SILInstruction.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4962,6 +4962,8 @@ class AssignOrInitInst
49624962
SILValue getInitializer() const { return Operands[2].get(); }
49634963
SILValue getSetter() { return Operands[3].get(); }
49644964

4965+
VarDecl *getProperty() const;
4966+
49654967
Mode getMode() const {
49664968
return Mode(sharedUInt8().AssignOrInitInst.mode);
49674969
}

‎lib/SIL/IR/SILInstructions.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,6 +1283,12 @@ bool AssignOrInitInst::isPropertyAlreadyInitialized(unsigned propertyIdx) {
12831283
return Assignments.test(propertyIdx);
12841284
}
12851285

1286+
VarDecl *AssignOrInitInst::getProperty() const {
1287+
auto *accessor = getReferencedInitAccessor();
1288+
assert(accessor);
1289+
return cast<VarDecl>(accessor->getStorage());
1290+
}
1291+
12861292
StringRef AssignOrInitInst::getPropertyName() const {
12871293
auto *accessor = getReferencedInitAccessor();
12881294
assert(accessor);

‎lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,11 @@ static unsigned getElementCountRec(TypeExpansionContext context,
9595
for (auto *VD : NTD->getStoredProperties())
9696
NumElements += getElementCountRec(
9797
context, Module, T.getFieldType(VD, Module, context), false);
98+
for (auto *P : NTD->getInitAccessorProperties()) {
99+
auto *init = P->getAccessor(AccessorKind::Init);
100+
if (init->getInitializedProperties().empty())
101+
++NumElements;
102+
}
98103
return NumElements;
99104
}
100105
}
@@ -451,6 +456,15 @@ DIMemoryObjectInfo::getPathStringToElement(unsigned Element,
451456
Element -= NumFieldElements;
452457
}
453458

459+
for (auto *property : NTD->getInitAccessorProperties()) {
460+
auto *init = property->getAccessor(AccessorKind::Init);
461+
if (init->getInitializedProperties().empty()) {
462+
if (Element == 0)
463+
return property;
464+
--Element;
465+
}
466+
}
467+
454468
// If we do not have any stored properties, we have nothing of interest.
455469
if (!HasStoredProperty)
456470
return nullptr;
@@ -497,6 +511,15 @@ bool DIMemoryObjectInfo::isElementLetProperty(unsigned Element) const {
497511
Element -= NumFieldElements;
498512
}
499513

514+
for (auto *property : NTD->getInitAccessorProperties()) {
515+
auto *init = property->getAccessor(AccessorKind::Init);
516+
if (init->getInitializedProperties().empty()) {
517+
if (Element == 0)
518+
return !property->getAccessor(AccessorKind::Set);
519+
--Element;
520+
}
521+
}
522+
500523
// Otherwise, we miscounted elements?
501524
assert(Element == 0 && "Element count problem");
502525
return false;
@@ -1221,10 +1244,22 @@ ElementUseCollector::collectAssignOrInitUses(AssignOrInitInst *Inst,
12211244

12221245
auto initializedElts = Inst->getInitializedProperties();
12231246
if (initializedElts.empty()) {
1224-
// Add a placeholder use that doesn't touch elements to make sure that
1225-
// the `assign_or_init` instruction gets the kind set when `initializes`
1226-
// list is empty.
1227-
trackUse(DIMemoryUse(Inst, DIUseKind::InitOrAssign, BaseEltNo, 0));
1247+
auto initAccessorProperties = typeDC->getInitAccessorProperties();
1248+
auto initFieldAt = typeDC->getStoredProperties().size();
1249+
1250+
for (auto *property : initAccessorProperties) {
1251+
auto initAccessor = property->getAccessor(AccessorKind::Init);
1252+
if (!initAccessor->getInitializedProperties().empty())
1253+
continue;
1254+
1255+
if (property == Inst->getProperty()) {
1256+
trackUse(DIMemoryUse(Inst, DIUseKind::InitOrAssign, initFieldAt,
1257+
/*NumElements=*/1));
1258+
break;
1259+
}
1260+
1261+
++initFieldAt;
1262+
}
12281263
} else {
12291264
for (auto *property : initializedElts)
12301265
addUse(property, DIUseKind::InitOrAssign);

‎lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,6 +1182,12 @@ void LifetimeChecker::doIt() {
11821182
while (returnBB != F.end()) {
11831183
auto *terminator = returnBB->getTerminator();
11841184

1185+
// If this is an unreachable block, let's ignore it.
1186+
if (isa<UnreachableInst>(terminator)) {
1187+
++returnBB;
1188+
continue;
1189+
}
1190+
11851191
if (!isInitializedAtUse(DIMemoryUse(terminator, DIUseKind::Load, 0, 1)))
11861192
diagnoseMissingInit();
11871193

‎lib/Sema/CodeSynthesis.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -877,12 +877,19 @@ bool AreAllStoredPropertiesDefaultInitableRequest::evaluate(
877877
if (llvm::any_of(initAccessorProperties, [&](const auto &entry) {
878878
auto *property =
879879
entry.second->getParentPatternBinding();
880-
return property->isInitialized(0);
880+
return property->isInitialized(0) ||
881+
property->isDefaultInitializable();
881882
}))
882883
return;
883884

884885
if (VD->hasStorageOrWrapsStorage())
885886
HasStorage = true;
887+
888+
// Treat an init accessor property that doesn't initialize other
889+
// properties as stored for initialization purposes.
890+
if (auto *initAccessor = VD->getAccessor(AccessorKind::Init)) {
891+
HasStorage |= initAccessor->getInitializedProperties().empty();
892+
}
886893
});
887894

888895
if (!HasStorage) continue;

‎lib/Sema/TypeCheckStorage.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -491,11 +491,16 @@ const PatternBindingEntry *PatternBindingEntryRequest::evaluate(
491491
}
492492
}
493493

494+
auto isInitAccessorProperty = [](Pattern *pattern) {
495+
auto *var = pattern->getSingleVar();
496+
return var && var->getAccessor(AccessorKind::Init);
497+
};
498+
494499
// If we have a type but no initializer, check whether the type is
495500
// default-initializable. If so, do it.
496501
if (!pbe.isInitialized() &&
497502
binding->isDefaultInitializable(entryNumber) &&
498-
pattern->hasStorage()) {
503+
(pattern->hasStorage() || isInitAccessorProperty(pattern))) {
499504
if (auto defaultInit = TypeChecker::buildDefaultInitializer(patternType)) {
500505
// If we got a default initializer, install it and re-type-check it
501506
// to make sure it is properly coerced to the pattern type.

‎test/Interpreter/init_accessors.swift

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -749,3 +749,100 @@ func test_inheritance() {
749749
test_inheritance()
750750
// CHECK: test-inheritance-1: Person(firstName: <<unknown>>, age: 0)
751751
// CHECK-NEXT: test-inheritance-2: Person(firstName: Q, age: 42)
752+
753+
do {
754+
class BackingData<T> {
755+
var data: [PartialKeyPath<T>: Any] = [:]
756+
757+
func get<V>(_ key: KeyPath<T, V>) -> V { data[key] as! V }
758+
func set<V>(_ key: KeyPath<T, V>, _ value: V) {
759+
data[key] = value
760+
}
761+
}
762+
763+
class Person : CustomStringConvertible {
764+
var description: String {
765+
"Person(name: \(name))"
766+
}
767+
768+
private var backingData: BackingData<Person> = BackingData()
769+
770+
private var _name: Int
771+
772+
var name: String {
773+
@storageRestrictions(accesses: backingData, initializes: _name)
774+
init(newValue) {
775+
self.backingData.set(\.name, newValue)
776+
self._name = 0
777+
}
778+
779+
get { self.backingData.get(\.name) }
780+
set { self.backingData.set(\.name, newValue) }
781+
}
782+
783+
init(name: String) {
784+
self.name = name
785+
}
786+
787+
init(backingData: BackingData<Person>) {
788+
self.backingData = backingData
789+
self._name = 0
790+
}
791+
}
792+
793+
let person = Person(name: "P")
794+
print(person)
795+
796+
let localData = BackingData<Person>()
797+
localData.set(\.name, "O")
798+
799+
print(Person(backingData: localData))
800+
}
801+
// CHECK: Person(name: P)
802+
// CHECK-NEXT: Person(name: O)
803+
804+
do {
805+
struct TestDefaultInitializable : CustomStringConvertible {
806+
var description: String {
807+
"TestDefaultInitializable(a: \(a))"
808+
}
809+
810+
var _a: Int?
811+
var a: Int? {
812+
@storageRestrictions(initializes: _a)
813+
init { _a = newValue }
814+
get { _a }
815+
}
816+
}
817+
818+
print(TestDefaultInitializable())
819+
print(TestDefaultInitializable(a: 42))
820+
821+
struct TestMixedDefaultInitalizable : CustomStringConvertible {
822+
var description: String {
823+
"TestMixedDefaultInitalizable(a: \(a), b: \(b))"
824+
}
825+
826+
var a: Int? {
827+
init {}
828+
get { nil }
829+
}
830+
831+
var _b: String
832+
var b: String? {
833+
@storageRestrictions(initializes: _b)
834+
init { self._b = (newValue ?? "") }
835+
get { _b }
836+
set { _b = newValue ?? "" }
837+
}
838+
}
839+
840+
print(TestMixedDefaultInitalizable())
841+
print(TestMixedDefaultInitalizable(b: "Hello"))
842+
print(TestMixedDefaultInitalizable(a: 42))
843+
}
844+
// CHECK: TestDefaultInitializable(a: nil)
845+
// CHECK-NEXT: TestDefaultInitializable(a: Optional(42))
846+
// CHECK-NEXT: TestMixedDefaultInitalizable(a: nil, b: Optional(""))
847+
// CHECK-NEXT: TestMixedDefaultInitalizable(a: nil, b: Optional("Hello"))
848+
// CHECK-NEXT: TestMixedDefaultInitalizable(a: nil, b: Optional(""))

‎test/SILOptimizer/init_accessor_definite_init_diagnostics.swift

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,17 @@ class TestInitWithGuard {
189189
}
190190
}
191191

192+
do {
193+
struct TestPropertyInitWithUnreachableBlocks {
194+
var _a: Int
195+
var a: Int? {
196+
@storageRestrictions(initializes: _a)
197+
init { _a = newValue ?? 0 } // Ok
198+
get { 42 }
199+
}
200+
}
201+
}
202+
192203
do {
193204
class Base<T: Collection> {
194205
private var _v: T
@@ -268,3 +279,73 @@ do {
268279
}
269280
}
270281
}
282+
283+
// Test that init accessor without "initializes" is required
284+
do {
285+
struct Test {
286+
var a: Int {
287+
init {}
288+
get { 42 }
289+
set {}
290+
}
291+
var b: Int { // expected-note 2 {{'self' not initialized}}
292+
init {}
293+
get { 0 }
294+
set { }
295+
}
296+
297+
init(a: Int) {
298+
self.a = a
299+
} // expected-error {{return from initializer without initializing all stored properties}}
300+
301+
init(a: Int, b: Int) {
302+
if a == 0 {
303+
self.a = a
304+
self.b = b
305+
} else {
306+
self.a = 0
307+
}
308+
} // expected-error {{return from initializer without initializing all stored properties}}
309+
310+
init() { // Ok
311+
self.a = 0
312+
self.b = 0
313+
}
314+
}
315+
316+
struct TestWithStored {
317+
var _value: Int = 0
318+
319+
var a: Int {
320+
@storageRestrictions(accesses: _value)
321+
init {}
322+
get { _value }
323+
set { _value = newValue }
324+
}
325+
}
326+
327+
_ = TestWithStored(a: 42) // Ok
328+
_ = TestWithStored(_value: 1, a: 42) // Ok
329+
330+
class TestWithStoredExplicit {
331+
var _value: Int = 0
332+
var _other: String = ""
333+
334+
var a: Int { // expected-note 2 {{'self' not initialized}}
335+
@storageRestrictions(accesses: _value)
336+
init {}
337+
get { _value }
338+
set { _value = newValue }
339+
}
340+
341+
init(data: Int) {
342+
self._value = data
343+
} // expected-error {{return from initializer without initializing all stored properties}}
344+
345+
init() {} // expected-error {{return from initializer without initializing all stored properties}}
346+
347+
init(a: Int) {
348+
self.a = a // Ok
349+
}
350+
}
351+
}

‎test/SILOptimizer/init_accessor_raw_sil_lowering.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,11 @@ final class TestIndirectionThroughStorage {
4646
// CHECK: [[STORAGE_INIT:%.*]] = function_ref @$s23assign_or_init_lowering29TestIndirectionThroughStorageC8_storage33_DE106275C2F16FB3D05881E72FBD87C8LLAA0H0_pAC1TAaFPRts_XPvpfi : $@convention(thin) () -> @out any Storage<TestIndirectionThroughStorage>
4747
// CHECK-NEXT: {{.*}} = apply [[STORAGE_INIT]]([[STORAGE_REF]]) : $@convention(thin) () -> @out any Storage<TestIndirectionThroughStorage>
4848
// Initialization:
49-
// CHECK: assign_or_init [set] self %2 : $TestIndirectionThroughStorage, value {{.*}} : $String, init {{.*}} : $@convention(thin) (@owned String, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (@owned String) -> ()
50-
// CHECK: assign_or_init [set] self %2 : $TestIndirectionThroughStorage, value {{.*}} : $Optional<Int>, init {{.*}} : $@convention(thin) (Optional<Int>, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (Optional<Int>) -> (
49+
// CHECK: assign_or_init [init] self %2 : $TestIndirectionThroughStorage, value {{.*}} : $String, init {{.*}} : $@convention(thin) (@owned String, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (@owned String) -> ()
50+
// CHECK: assign_or_init [init] self %2 : $TestIndirectionThroughStorage, value {{.*}} : $Optional<Int>, init {{.*}} : $@convention(thin) (Optional<Int>, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (Optional<Int>) -> ()
5151
// Explicit set:
5252
// CHECK: assign_or_init [set] self %2 : $TestIndirectionThroughStorage, value {{.*}} : $String, init {{.*}} : $@convention(thin) (@owned String, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (@owned String) -> ()
53-
// CHECK: assign_or_init [set] self %2 : $TestIndirectionThroughStorage, value {{.*}} : $Optional<Int>, init {{.*}} : $@convention(thin) (Optional<Int>, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (Optional<Int>) -> (
53+
// CHECK: assign_or_init [set] self %2 : $TestIndirectionThroughStorage, value {{.*}} : $Optional<Int>, init {{.*}} : $@convention(thin) (Optional<Int>, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (Optional<Int>) -> ()
5454
init(name: String, age: Int) {
5555
self.name = name
5656
self.age = age
@@ -61,8 +61,8 @@ final class TestIndirectionThroughStorage {
6161
// CHECK: [[STORAGE_INIT:%.*]] = function_ref @$s23assign_or_init_lowering29TestIndirectionThroughStorageC8_storage33_DE106275C2F16FB3D05881E72FBD87C8LLAA0H0_pAC1TAaFPRts_XPvpfi : $@convention(thin) () -> @out any Storage<TestIndirectionThroughStorage>
6262
// CHECK-NEXT: {{.*}} = apply [[STORAGE_INIT]]([[STORAGE_REF]]) : $@convention(thin) () -> @out any Storage<TestIndirectionThroughStorage>
6363
// Initialization:
64-
// CHECK: assign_or_init [set] self %1 : $TestIndirectionThroughStorage, value {{.*}} : $String, init {{.*}} : $@convention(thin) (@owned String, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (@owned String) -> ()
65-
// CHECK: assign_or_init [set] self %1 : $TestIndirectionThroughStorage, value {{.*}} : $Optional<Int>, init {{.*}} : $@convention(thin) (Optional<Int>, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (Optional<Int>) -> ()
64+
// CHECK: assign_or_init [init] self %1 : $TestIndirectionThroughStorage, value {{.*}} : $String, init {{.*}} : $@convention(thin) (@owned String, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (@owned String) -> ()
65+
// CHECK: assign_or_init [init] self %1 : $TestIndirectionThroughStorage, value {{.*}} : $Optional<Int>, init {{.*}} : $@convention(thin) (Optional<Int>, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (Optional<Int>) -> ()
6666
// Explicit set:
6767
// CHECK: [[STORAGE_SETTER:%.*]] = function_ref @$s23assign_or_init_lowering29TestIndirectionThroughStorageC7storageAA0H0_pAC1TAaEPRts_XPvs : $@convention(method) (@in any Storage<TestIndirectionThroughStorage>, @guaranteed TestIndirectionThroughStorage) -> ()
6868
// CHECK-NEXT: {{.*}} = apply [[STORAGE_SETTER]]({{.*}}, %1) : $@convention(method) (@in any Storage<TestIndirectionThroughStorage>, @guaranteed TestIndirectionThroughStorage) -> ()
@@ -102,8 +102,8 @@ struct TestAccessOfOnePatternVars {
102102
// CHECK-NEXT: {{.*}} = apply [[Y_INIT]]() : $@convention(thin) () -> @owned String
103103
// CHECK-NOT: [[X_REF:%.*]] = struct_element_addr %3 : $*TestAccessOfOnePatternVars, #TestAccessOfOnePatternVars.x
104104
// CHECK-NOT: [[Y_REF:%.*]] = struct_element_addr {{.*}} : $*TestAccessOfOnePatternVars, #TestAccessOfOnePatternVars.y
105-
// CHECK: assign_or_init [set] self {{.*}} : $*TestAccessOfOnePatternVars, value {{.*}} : $(Int, String), init {{.*}} : $@convention(thin) (Int, @owned String, @inout Int, @inout String) -> (), set {{.*}} : $@callee_guaranteed (Int, @owned String) -> ()
106-
// CHECK: assign_or_init [set] self {{.*}} : $*TestAccessOfOnePatternVars, value {{.*}} : $Bool, init {{.*}} : $@convention(thin) (Bool, @inout Int) -> (), set {{.*}} : $@callee_guaranteed (Bool) -> ()
105+
// CHECK: assign_or_init [init] self {{.*}} : $*TestAccessOfOnePatternVars, value {{.*}} : $(Int, String), init {{.*}} : $@convention(thin) (Int, @owned String, @inout Int, @inout String) -> (), set {{.*}} : $@callee_guaranteed (Int, @owned String) -> ()
106+
// CHECK: assign_or_init [init] self {{.*}} : $*TestAccessOfOnePatternVars, value {{.*}} : $Bool, init {{.*}} : $@convention(thin) (Bool, @inout Int) -> (), set {{.*}} : $@callee_guaranteed (Bool) -> ()
107107
init(x: Int, y: String) {
108108
self.x = x
109109
self.y = y
@@ -196,7 +196,7 @@ struct Test {
196196
// CHECK-LABEL: sil hidden [ossa] @$s23assign_or_init_lowering4TestV1vACSi_tcfC : $@convention(method) (Int, @thin Test.Type) -> Test
197197
init(v: Int) {
198198
// CHECK: [[INIT_REF:%.*]] = function_ref @$s23assign_or_init_lowering4TestV4testSivi : $@convention(thin) (Int) -> ()
199-
// CHECK: assign_or_init [set] self {{.*}}, value %0 : $Int, init [[INIT_REF]] : $@convention(thin) (Int) -> (), set {{.*}} : $@callee_guaranteed (Int) -> ()
199+
// CHECK: assign_or_init [init] self {{.*}}, value %0 : $Int, init [[INIT_REF]] : $@convention(thin) (Int) -> (), set {{.*}} : $@callee_guaranteed (Int) -> ()
200200
self.test = v
201201
}
202202
}

‎test/SILOptimizer/init_accessors.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,12 @@ struct TestSetter {
7373
}
7474

7575
// CHECK-LABEL: sil hidden [ossa] @$s14init_accessors10TestSetterV1x1yACSi_SitcfC : $@convention(method) (Int, Int, @thin TestSetter.Type) -> TestSetter
76-
// CHECK: [[SETTER_REF:%.*]] = function_ref @$s14init_accessors10TestSetterV5pointSi_Sitvs : $@convention(method) (Int, Int, @inout TestSetter) -> ()
77-
// CHECK-NEXT: [[SETTER_CLOSURE:%.*]] = partial_apply [callee_guaranteed] [[SETTER_REF]]([[SELF_VALUE:%.*]]) : $@convention(method) (Int, Int, @inout TestSetter) -> ()
78-
// CHECK: {{.*}} = apply [[SETTER_CLOSURE]]({{.*}}) : $@callee_guaranteed (Int, Int) -> ()
76+
// CHECK: [[INIT_ACCESSOR:%.*]] = function_ref @$s14init_accessors10TestSetterV5pointSi_Sitvi : $@convention(thin) (Int, Int, @inout Int, @inout Int) -> ()
77+
// CHECK: [[SELF:%.*]] = begin_access [modify] [dynamic] %14 : $*TestSetter
78+
// CHECK-NEXT: ([[X:%.*]], [[Y:%.*]]) = destructure_tuple {{.*}} : $(Int, Int)
79+
// CHECK-NEXT: [[X_REF:%.*]] = struct_element_addr [[SELF]] : $*TestSetter, #TestSetter.x
80+
// CHECK-NEXT: [[Y_REF:%.*]] = struct_element_addr [[SELF]] : $*TestSetter, #TestSetter.y
81+
// CHECK-NEXT: {{.*}} = apply [[INIT_ACCESSOR]]([[X]], [[Y]], [[X_REF]], [[Y_REF]]) : $@convention(thin) (Int, Int, @inout Int, @inout Int) -> ()
7982
init(x: Int, y: Int) {
8083
self.x = x
8184
self.y = y

0 commit comments

Comments
 (0)