Skip to content

Commit 5aed2fc

Browse files
xedinxedin
authored
Merge pull request #67586 from xedin/rdar-112984795-5.9
[5.9][SILGenConstructor] InitAccessors: Make sure that accessed fields are initialized before init accessors
2 parents e02171d + ffeae0e commit 5aed2fc

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
@@ -1502,87 +1502,109 @@ void SILGenFunction::emitMemberInitializationViaInitAccessor(
15021502
B.createEndAccess(loc, selfRef.getValue(), /*aborted=*/false);
15031503
}
15041504

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

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

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

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

‎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)