Skip to content

Commit 8af81a3

Browse files
xedinxedin
authored
Merge pull request #67585 from xedin/rdar-112984795
[SILGenConstructor] InitAccessors: Make sure that accessed fields are initialized before init accessors
2 parents 992398b + f6017cc commit 8af81a3

File tree

3 files changed

+205
-63
lines changed

3 files changed

+205
-63
lines changed

‎lib/SILGen/SILGenConstructor.cpp‎

Lines changed: 85 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1503,87 +1503,109 @@ void SILGenFunction::emitMemberInitializationViaInitAccessor(
15031503
B.createEndAccess(loc, selfRef.getValue(), /*aborted=*/false);
15041504
}
15051505

1506+
void SILGenFunction::emitMemberInitializer(DeclContext *dc, VarDecl *selfDecl,
1507+
PatternBindingDecl *field,
1508+
SubstitutionMap substitutions) {
1509+
assert(!field->isStatic());
1510+
1511+
for (auto i : range(field->getNumPatternEntries())) {
1512+
auto init = field->getExecutableInit(i);
1513+
if (!init)
1514+
continue;
1515+
1516+
auto *varPattern = field->getPattern(i);
1517+
1518+
// Cleanup after this initialization.
1519+
FullExpr scope(Cleanups, varPattern);
1520+
1521+
// Get the type of the initialization result, in terms
1522+
// of the constructor context's archetypes.
1523+
auto resultType =
1524+
getInitializationTypeInContext(field->getDeclContext(), dc, varPattern);
1525+
AbstractionPattern origType = resultType.first;
1526+
CanType substType = resultType.second;
1527+
1528+
// Figure out what we're initializing.
1529+
auto memberInit = emitMemberInit(*this, selfDecl, varPattern);
1530+
1531+
// This whole conversion thing is about eliminating the
1532+
// paired orig-to-subst subst-to-orig conversions that
1533+
// will happen if the storage is at a different abstraction
1534+
// level than the constructor. When emitApply() is used
1535+
// to call the stored property initializer, it naturally
1536+
// wants to convert the result back to the most substituted
1537+
// abstraction level. To undo this, we use a converting
1538+
// initialization and rely on the peephole that optimizes
1539+
// out the redundant conversion.
1540+
SILType loweredResultTy;
1541+
SILType loweredSubstTy;
1542+
1543+
// A converting initialization isn't necessary if the member is
1544+
// a property wrapper. Though the initial value can have a
1545+
// reabstractable type, the result of the initialization is
1546+
// always the property wrapper type, which is never reabstractable.
1547+
bool needsConvertingInit = false;
1548+
auto *singleVar = varPattern->getSingleVar();
1549+
if (!(singleVar && singleVar->getOriginalWrappedProperty())) {
1550+
loweredResultTy = getLoweredType(origType, substType);
1551+
loweredSubstTy = getLoweredType(substType);
1552+
needsConvertingInit = loweredResultTy != loweredSubstTy;
1553+
}
1554+
1555+
if (needsConvertingInit) {
1556+
Conversion conversion =
1557+
Conversion::getSubstToOrig(origType, substType, loweredResultTy);
1558+
1559+
ConvertingInitialization convertingInit(conversion,
1560+
SGFContext(memberInit.get()));
1561+
1562+
emitAndStoreInitialValueInto(*this, varPattern, field, i, substitutions,
1563+
origType, substType, &convertingInit);
1564+
1565+
auto finalValue = convertingInit.finishEmission(
1566+
*this, varPattern, ManagedValue::forInContext());
1567+
if (!finalValue.isInContext())
1568+
finalValue.forwardInto(*this, varPattern, memberInit.get());
1569+
} else {
1570+
emitAndStoreInitialValueInto(*this, varPattern, field, i, substitutions,
1571+
origType, substType, memberInit.get());
1572+
}
1573+
}
1574+
}
1575+
15061576
void SILGenFunction::emitMemberInitializers(DeclContext *dc,
15071577
VarDecl *selfDecl,
15081578
NominalTypeDecl *nominal) {
15091579
auto subs = getSubstitutionsForPropertyInitializer(dc, nominal);
15101580

1581+
llvm::SmallPtrSet<PatternBindingDecl *, 4> alreadyInitialized;
15111582
for (auto member : nominal->getImplementationContext()->getAllMembers()) {
15121583
// Find instance pattern binding declarations that have initializers.
15131584
if (auto pbd = dyn_cast<PatternBindingDecl>(member)) {
15141585
if (pbd->isStatic()) continue;
15151586

1587+
if (alreadyInitialized.count(pbd))
1588+
continue;
1589+
15161590
// Emit default initialization for an init accessor property.
15171591
if (auto *var = pbd->getSingleVar()) {
15181592
if (var->hasInitAccessor()) {
1593+
auto initAccessor = var->getAccessor(AccessorKind::Init);
1594+
1595+
// Make sure that initializations for the accessed properties
1596+
// are emitted before the init accessor that uses them.
1597+
for (auto *property : initAccessor->getAccessedProperties()) {
1598+
auto *PBD = property->getParentPatternBinding();
1599+
if (alreadyInitialized.insert(PBD).second)
1600+
emitMemberInitializer(dc, selfDecl, PBD, subs);
1601+
}
1602+
15191603
emitMemberInitializationViaInitAccessor(dc, selfDecl, pbd, subs);
15201604
continue;
15211605
}
15221606
}
15231607

1524-
for (auto i : range(pbd->getNumPatternEntries())) {
1525-
auto init = pbd->getExecutableInit(i);
1526-
if (!init) continue;
1527-
1528-
auto *varPattern = pbd->getPattern(i);
1529-
1530-
// Cleanup after this initialization.
1531-
FullExpr scope(Cleanups, varPattern);
1532-
1533-
// Get the type of the initialization result, in terms
1534-
// of the constructor context's archetypes.
1535-
auto resultType = getInitializationTypeInContext(
1536-
pbd->getDeclContext(), dc, varPattern);
1537-
AbstractionPattern origType = resultType.first;
1538-
CanType substType = resultType.second;
1539-
1540-
// Figure out what we're initializing.
1541-
auto memberInit = emitMemberInit(*this, selfDecl, varPattern);
1542-
1543-
// This whole conversion thing is about eliminating the
1544-
// paired orig-to-subst subst-to-orig conversions that
1545-
// will happen if the storage is at a different abstraction
1546-
// level than the constructor. When emitApply() is used
1547-
// to call the stored property initializer, it naturally
1548-
// wants to convert the result back to the most substituted
1549-
// abstraction level. To undo this, we use a converting
1550-
// initialization and rely on the peephole that optimizes
1551-
// out the redundant conversion.
1552-
SILType loweredResultTy;
1553-
SILType loweredSubstTy;
1554-
1555-
// A converting initialization isn't necessary if the member is
1556-
// a property wrapper. Though the initial value can have a
1557-
// reabstractable type, the result of the initialization is
1558-
// always the property wrapper type, which is never reabstractable.
1559-
bool needsConvertingInit = false;
1560-
auto *singleVar = varPattern->getSingleVar();
1561-
if (!(singleVar && singleVar->getOriginalWrappedProperty())) {
1562-
loweredResultTy = getLoweredType(origType, substType);
1563-
loweredSubstTy = getLoweredType(substType);
1564-
needsConvertingInit = loweredResultTy != loweredSubstTy;
1565-
}
1566-
1567-
if (needsConvertingInit) {
1568-
Conversion conversion = Conversion::getSubstToOrig(
1569-
origType, substType,
1570-
loweredResultTy);
1571-
1572-
ConvertingInitialization convertingInit(conversion,
1573-
SGFContext(memberInit.get()));
1574-
1575-
emitAndStoreInitialValueInto(*this, varPattern, pbd, i, subs,
1576-
origType, substType, &convertingInit);
1577-
1578-
auto finalValue = convertingInit.finishEmission(
1579-
*this, varPattern, ManagedValue::forInContext());
1580-
if (!finalValue.isInContext())
1581-
finalValue.forwardInto(*this, varPattern, memberInit.get());
1582-
} else {
1583-
emitAndStoreInitialValueInto(*this, varPattern, pbd, i, subs,
1584-
origType, substType, memberInit.get());
1585-
}
1586-
}
1608+
emitMemberInitializer(dc, selfDecl, pbd, subs);
15871609
}
15881610
}
15891611
}

‎lib/SILGen/SILGenFunction.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,6 +802,17 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
802802
void emitMemberInitializers(DeclContext *dc, VarDecl *selfDecl,
803803
NominalTypeDecl *nominal);
804804

805+
/// Generates code to initialize stored property from its
806+
/// initializer.
807+
///
808+
/// \param dc The DeclContext containing the current function.
809+
/// \param selfDecl The 'self' declaration within the current function.
810+
/// \param field The stored property that has to be initialized.
811+
/// \param substitutions The substitutions to apply to initializer and setter.
812+
void emitMemberInitializer(DeclContext *dc, VarDecl *selfDecl,
813+
PatternBindingDecl *field,
814+
SubstitutionMap substitutions);
815+
805816
void emitMemberInitializationViaInitAccessor(DeclContext *dc,
806817
VarDecl *selfDecl,
807818
PatternBindingDecl *member,

‎test/SILOptimizer/init_accessor_raw_sil_lowering.swift‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,115 @@
22

33
// REQUIRES: asserts
44

5+
protocol Storage<T> {
6+
associatedtype T
7+
8+
func getValue<V>(_: KeyPath<T, V>) -> V
9+
func setValue<V>(_: KeyPath<T, V>, _: V)
10+
}
11+
12+
struct NoopStorage<T>: Storage {
13+
init() {}
14+
15+
func getValue<V>(_: KeyPath<T, V>) -> V { fatalError() }
16+
func setValue<V>(_: KeyPath<T, V>, _: V) {}
17+
}
18+
19+
final class TestIndirectionThroughStorage {
20+
var name: String = "item1" {
21+
@storageRestrictions(accesses: _storage)
22+
init {
23+
_storage.setValue(\.name, newValue)
24+
}
25+
get { _storage.getValue(\.name) }
26+
set { }
27+
}
28+
29+
var age: Int? = nil {
30+
@storageRestrictions(accesses: _storage)
31+
init {
32+
_storage.setValue(\.age, newValue)
33+
}
34+
35+
get { _storage.getValue(\.age) }
36+
set { }
37+
}
38+
39+
private var _storage: any Storage<TestIndirectionThroughStorage> = NoopStorage()
40+
41+
var storage: any Storage<TestIndirectionThroughStorage> {
42+
get { _storage }
43+
set { _storage = newValue }
44+
}
45+
46+
// CHECK-LABEL: sil hidden [ossa] @$s23assign_or_init_lowering29TestIndirectionThroughStorageC4name3ageACSS_Sitcfc : $@convention(method) (@owned String, Int, @owned TestIndirectionThroughStorage) -> @owned TestIndirectionThroughStorage
47+
// CHECK: [[STORAGE_REF:%.*]] = ref_element_addr {{.*}} : $TestIndirectionThroughStorage, #TestIndirectionThroughStorage._storage
48+
// CHECK: [[STORAGE_INIT:%.*]] = function_ref @$s23assign_or_init_lowering29TestIndirectionThroughStorageC8_storage33_DE106275C2F16FB3D05881E72FBD87C8LLAA0H0_pAC1TAaFPRts_XPvpfi : $@convention(thin) () -> @out any Storage<TestIndirectionThroughStorage>
49+
// CHECK-NEXT: {{.*}} = apply [[STORAGE_INIT]]([[STORAGE_REF]]) : $@convention(thin) () -> @out any Storage<TestIndirectionThroughStorage>
50+
// Initialization:
51+
// CHECK: assign_or_init [set] self %2 : $TestIndirectionThroughStorage, value {{.*}} : $String, init {{.*}} : $@convention(thin) (@owned String, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (@owned String) -> ()
52+
// 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+
// Explicit set:
54+
// CHECK: assign_or_init [set] self %2 : $TestIndirectionThroughStorage, value {{.*}} : $String, init {{.*}} : $@convention(thin) (@owned String, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (@owned String) -> ()
55+
// 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>) -> (
56+
init(name: String, age: Int) {
57+
self.name = name
58+
self.age = age
59+
}
60+
61+
// CHECK-LABEL: sil hidden [ossa] @$s23assign_or_init_lowering29TestIndirectionThroughStorageC7storageAcA0H0_pAC1TAaEPRts_XP_tcfc : $@convention(method) (@in any Storage<TestIndirectionThroughStorage>, @owned TestIndirectionThroughStorage) -> @owned TestIndirectionThroughStorage
62+
// CHECK: [[STORAGE_REF:%.*]] = ref_element_addr {{.*}} : $TestIndirectionThroughStorage, #TestIndirectionThroughStorage._storage
63+
// CHECK: [[STORAGE_INIT:%.*]] = function_ref @$s23assign_or_init_lowering29TestIndirectionThroughStorageC8_storage33_DE106275C2F16FB3D05881E72FBD87C8LLAA0H0_pAC1TAaFPRts_XPvpfi : $@convention(thin) () -> @out any Storage<TestIndirectionThroughStorage>
64+
// CHECK-NEXT: {{.*}} = apply [[STORAGE_INIT]]([[STORAGE_REF]]) : $@convention(thin) () -> @out any Storage<TestIndirectionThroughStorage>
65+
// Initialization:
66+
// CHECK: assign_or_init [set] self %1 : $TestIndirectionThroughStorage, value {{.*}} : $String, init {{.*}} : $@convention(thin) (@owned String, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (@owned String) -> ()
67+
// 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>) -> ()
68+
// Explicit set:
69+
// CHECK: [[STORAGE_SETTER:%.*]] = function_ref @$s23assign_or_init_lowering29TestIndirectionThroughStorageC7storageAA0H0_pAC1TAaEPRts_XPvs : $@convention(method) (@in any Storage<TestIndirectionThroughStorage>, @guaranteed TestIndirectionThroughStorage) -> ()
70+
// CHECK-NEXT: {{.*}} = apply [[STORAGE_SETTER]]({{.*}}, %1) : $@convention(method) (@in any Storage<TestIndirectionThroughStorage>, @guaranteed TestIndirectionThroughStorage) -> ()
71+
init(storage: any Storage<TestIndirectionThroughStorage>) {
72+
self.storage = storage
73+
}
74+
}
75+
76+
struct TestAccessOfOnePatternVars {
77+
var data: (Int, String) = (0, "a") {
78+
@storageRestrictions(accesses: x, y)
79+
init {
80+
}
81+
82+
get { (x, y) }
83+
set {
84+
x = newValue.0
85+
y = newValue.1
86+
}
87+
}
88+
89+
var other: Bool = false {
90+
@storageRestrictions(accesses: x)
91+
init {}
92+
get { x != 0 }
93+
set {}
94+
}
95+
96+
var x: Int = 1, y: String = ""
97+
98+
// CHECK-LABEL: sil hidden [ossa] @$s23assign_or_init_lowering26TestAccessOfOnePatternVarsV1x1yACSi_SStcfC : $@convention(method) (Int, @owned String, @thin TestAccessOfOnePatternVars.Type) -> @owned TestAccessOfOnePatternVars
99+
// CHECK: [[X_REF:%.*]] = struct_element_addr {{.*}} : $*TestAccessOfOnePatternVars, #TestAccessOfOnePatternVars.x
100+
// CHECK: [[X_INIT:%.*]] = function_ref @$s23assign_or_init_lowering26TestAccessOfOnePatternVarsV1xSivpfi : $@convention(thin) () -> Int
101+
// CHECK-NEXT: {{.*}} = apply [[X_INIT]]() : $@convention(thin) () -> Int
102+
// CHECK: [[Y_REF:%.*]] = struct_element_addr {{.*}} : $*TestAccessOfOnePatternVars, #TestAccessOfOnePatternVars.y
103+
// CHECK: [[Y_INIT:%.*]] = function_ref @$s23assign_or_init_lowering26TestAccessOfOnePatternVarsV1ySSvpfi : $@convention(thin) () -> @owned String
104+
// CHECK-NEXT: {{.*}} = apply [[Y_INIT]]() : $@convention(thin) () -> @owned String
105+
// CHECK-NOT: [[X_REF:%.*]] = struct_element_addr %3 : $*TestAccessOfOnePatternVars, #TestAccessOfOnePatternVars.x
106+
// CHECK-NOT: [[Y_REF:%.*]] = struct_element_addr {{.*}} : $*TestAccessOfOnePatternVars, #TestAccessOfOnePatternVars.y
107+
// 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) -> ()
108+
// CHECK: assign_or_init [set] self {{.*}} : $*TestAccessOfOnePatternVars, value {{.*}} : $Bool, init {{.*}} : $@convention(thin) (Bool, @inout Int) -> (), set {{.*}} : $@callee_guaranteed (Bool) -> ()
109+
init(x: Int, y: String) {
110+
self.x = x
111+
self.y = y
112+
}
113+
}
5114

6115
struct Test1 {
7116
var _a: Int

0 commit comments

Comments
 (0)