Skip to content

Commit ddcfe01

Browse files
xedinxedin
committed
[Sema/SILGen] InitAccessors: Emit intersecting init accessor calls in memberwise init
If init accessor initialize the same properties, let's emit them in sequence and emit `destroy_addr` in-between to make sure that there is no double initialization.
1 parent f6fd0bc commit ddcfe01

File tree

5 files changed

+348
-30
lines changed

5 files changed

+348
-30
lines changed

Diff for: ‎lib/SILGen/SILGenConstructor.cpp

+35-15
Original file line numberDiff line numberDiff line change
@@ -272,22 +272,31 @@ static RValue maybeEmitPropertyWrapperInitFromValue(
272272
subs, std::move(arg));
273273
}
274274

275-
static void emitApplyOfInitAccessor(SILGenFunction &SGF, SILLocation loc,
276-
AccessorDecl *accessor, SILValue selfValue,
277-
SILType selfTy, RValue &&initialValue) {
275+
static void
276+
emitApplyOfInitAccessor(SILGenFunction &SGF, SILLocation loc,
277+
AccessorDecl *accessor, SILValue selfValue,
278+
SILType selfTy, RValue &&initialValue,
279+
llvm::SmallPtrSetImpl<VarDecl *> &initializedFields) {
278280
SmallVector<SILValue> arguments;
279281

280-
auto emitFieldReference = [&](VarDecl *field) {
282+
auto emitFieldReference = [&](VarDecl *field, bool forInit = false) {
281283
auto fieldTy =
282284
selfTy.getFieldType(field, SGF.SGM.M, SGF.getTypeExpansionContext());
283-
return SGF.B.createStructElementAddr(loc, selfValue, field,
284-
fieldTy.getAddressType());
285+
auto fieldAddr = SGF.B.createStructElementAddr(loc, selfValue, field,
286+
fieldTy.getAddressType());
287+
288+
// If another init accessor already initialized this field then we need
289+
// to emit destroy before calling this accessor.
290+
if (forInit && !initializedFields.insert(field).second)
291+
SGF.B.emitDestroyAddr(loc, fieldAddr);
292+
293+
return fieldAddr;
285294
};
286295

287296
// First, let's emit all of the indirect results.
288297
if (auto *initAttr = accessor->getAttrs().getAttribute<InitializesAttr>()) {
289298
for (auto *property : initAttr->getPropertyDecls(accessor)) {
290-
arguments.push_back(emitFieldReference(property));
299+
arguments.push_back(emitFieldReference(property, /*forInit=*/true));
291300
}
292301
}
293302

@@ -402,24 +411,35 @@ static void emitImplicitValueConstructor(SILGenFunction &SGF,
402411
// Tracks all the init accessors we have emitted
403412
// because they can initialize more than one property.
404413
llvm::SmallPtrSet<AccessorDecl *, 2> emittedInitAccessors;
414+
// Tracks all the fields that have been already initialized
415+
// via an init accessor.
416+
llvm::SmallPtrSet<VarDecl *, 4> fieldsInitializedViaAccessor;
405417

406418
auto elti = elements.begin(), eltEnd = elements.end();
407419
for (VarDecl *field : decl->getStoredProperties()) {
408420

409421
// Handle situations where this stored propery is initialized
410422
// via a call to an init accessor on some other property.
411423
if (initializedViaAccessor.count(field)) {
412-
auto *initProperty = initializedViaAccessor.find(field)->second;
413-
auto *initAccessor = initProperty->getAccessor(AccessorKind::Init);
424+
for (auto iter = initializedViaAccessor.find(field);
425+
iter != initializedViaAccessor.end(); ++iter) {
426+
auto *initProperty = iter->second;
427+
auto *initAccessor = initProperty->getAccessor(AccessorKind::Init);
414428

415-
if (emittedInitAccessors.count(initAccessor))
416-
continue;
429+
if (!emittedInitAccessors.insert(initAccessor).second)
430+
continue;
417431

418-
emitApplyOfInitAccessor(SGF, Loc, initAccessor, resultSlot, selfTy,
419-
std::move(*elti));
432+
assert(elti != eltEnd &&
433+
"number of args does not match number of fields");
420434

421-
emittedInitAccessors.insert(initAccessor);
422-
++elti;
435+
emitApplyOfInitAccessor(SGF, Loc, initAccessor, resultSlot, selfTy,
436+
std::move(*elti),
437+
fieldsInitializedViaAccessor);
438+
++elti;
439+
}
440+
441+
// After all init accessors are emitted let's move on to the next
442+
// property.
423443
continue;
424444
}
425445

Diff for: ‎lib/Sema/CodeSynthesis.cpp

+15-15
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,11 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl,
303303
std::multimap<VarDecl *, VarDecl *> initializedViaAccessor;
304304
decl->collectPropertiesInitializableByInitAccessors(initializedViaAccessor);
305305

306+
auto createParameter = [&](VarDecl *property) {
307+
accessLevel = std::min(accessLevel, property->getFormalAccess());
308+
params.push_back(createMemberwiseInitParameter(decl, Loc, property));
309+
};
310+
306311
// A single property could be used to initialize N other stored
307312
// properties via a call to its init accessor.
308313
llvm::SmallPtrSet<VarDecl *, 4> usedInitProperties;
@@ -315,22 +320,17 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl,
315320
continue;
316321

317322
// Check whether this property could be initialized via init accessor.
318-
//
319-
// Note that we check for a single match here because intersecting
320-
// properties are going to be diagnosed.
321-
if (initializedViaAccessor.count(var) == 1) {
322-
auto *initializerProperty = initializedViaAccessor.find(var)->second;
323-
// Parameter for this property is already emitted.
324-
if (usedInitProperties.count(initializerProperty))
325-
continue;
326-
327-
var = initializerProperty;
328-
usedInitProperties.insert(initializerProperty);
329-
}
330-
331-
accessLevel = std::min(accessLevel, var->getFormalAccess());
323+
if (initializedViaAccessor.count(var)) {
324+
for (auto iter = initializedViaAccessor.find(var);
325+
iter != initializedViaAccessor.end(); ++iter) {
326+
auto *initializerProperty = iter->second;
332327

333-
params.push_back(createMemberwiseInitParameter(decl, Loc, var));
328+
if (usedInitProperties.insert(initializerProperty).second)
329+
createParameter(initializerProperty);
330+
}
331+
} else {
332+
createParameter(var);
333+
}
334334
}
335335
} else if (ICK == ImplicitConstructorKind::DefaultDistributedActor) {
336336
auto classDecl = dyn_cast<ClassDecl>(decl);

Diff for: ‎test/Interpreter/init_accessors.swift

+122
Original file line numberDiff line numberDiff line change
@@ -332,3 +332,125 @@ test_assignments()
332332
// CHECK-NEXT: test-assignments-1: (3, 42)
333333
// CHECK-NEXT: a-init-accessor: 0
334334
// CHECK-NEXT: test-assignments-2: (0, 2)
335+
336+
func test_memberwise_with_overlaps() {
337+
struct Test1<T, U> {
338+
var _a: T
339+
var _b: Int
340+
341+
var a: T {
342+
init(initialValue) initializes(_a) {
343+
_a = initialValue
344+
}
345+
346+
get { _a }
347+
set { }
348+
}
349+
350+
var pair: (T, Int) {
351+
init(initialValue) initializes(_a, _b) {
352+
_a = initialValue.0
353+
_b = initialValue.1
354+
}
355+
356+
get { (_a, _b) }
357+
set { }
358+
}
359+
360+
var c: U
361+
}
362+
363+
let test1 = Test1(a: "a", pair: ("b", 1), c: [3.0])
364+
print("memberwise-overlaps-1: \(test1)")
365+
366+
struct Test2<T, U> {
367+
var _a: T
368+
var _b: Int
369+
370+
var a: T {
371+
init(initialValue) initializes(_a) {
372+
_a = initialValue
373+
}
374+
375+
get { _a }
376+
set { }
377+
}
378+
379+
var b: Int {
380+
init(initialValue) initializes(_b) {
381+
_b = initialValue
382+
}
383+
384+
get { _b }
385+
set { }
386+
}
387+
388+
var _c: U
389+
390+
var pair: (T, U) {
391+
init(initialValue) initializes(_a, _c) {
392+
_a = initialValue.0
393+
_c = initialValue.1
394+
}
395+
396+
get { (_a, _c) }
397+
set { }
398+
}
399+
}
400+
401+
let test2 = Test2(a: "a", pair: ("c", 2), b: 0)
402+
print("memberwise-overlaps-2: \(test2)")
403+
404+
struct Test3<T, U> {
405+
var _a: T
406+
var _b: Int
407+
408+
var a: T {
409+
init(initialValue) initializes(_a) {
410+
_a = initialValue
411+
}
412+
413+
get { _a }
414+
set { }
415+
}
416+
417+
var b: Int {
418+
init(initialValue) initializes(_b) {
419+
_b = initialValue
420+
}
421+
422+
get { _b }
423+
set { }
424+
}
425+
426+
var _c: U
427+
428+
var c: U {
429+
init(initialValue) initializes(_c) {
430+
_c = initialValue
431+
}
432+
433+
get { _c }
434+
set { }
435+
}
436+
437+
var triple: (T, Int, U) {
438+
init(initialValue) initializes(_a, _b, _c) {
439+
_a = initialValue.0
440+
_b = initialValue.1
441+
_c = initialValue.2
442+
}
443+
444+
get { (_a, _b, _c) }
445+
set { }
446+
}
447+
}
448+
449+
let test3 = Test3(a: "a", triple: ("b", 2, [1.0, 2.0]), b: 0, c: [1.0])
450+
print("memberwise-overlaps-3: \(test3)")
451+
}
452+
453+
test_memberwise_with_overlaps()
454+
// CHECK: memberwise-overlaps-1: Test1<String, Array<Double>>(_a: "b", _b: 1, c: [3.0])
455+
// CHECK-NEXT: memberwise-overlaps-2: Test2<String, Int>(_a: "c", _b: 0, _c: 2)
456+
// CHECK-NEXT: memberwise-overlaps-3: Test3<String, Array<Double>>(_a: "b", _b: 0, _c: [1.0])

Diff for: ‎test/SILOptimizer/init_accessor_raw_sil_lowering.swift

+62
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,68 @@
22

33
// REQUIRES: asserts
44

5+
// Makes sure that SILGen for memberwise initializer has destroy_addr for overlapping properties.
6+
struct TestMemberwiseWithOverlap {
7+
var _a: Int
8+
var _b: Int
9+
var _c: Int
10+
11+
var a: Int {
12+
init(initialValue) initializes(_a) {
13+
_a = initialValue
14+
}
15+
16+
get { _a }
17+
set { }
18+
}
19+
20+
var pair: (Int, Int) {
21+
init(initialValue) initializes(_a, _b) {
22+
_a = initialValue.0
23+
_b = initialValue.1
24+
}
25+
26+
get { (_a, _b) }
27+
set { }
28+
}
29+
30+
var c: Int {
31+
init(initialValue) initializes(_b, _c) accesses(_a) {
32+
_b = -1
33+
_c = initialValue
34+
}
35+
36+
get { _c }
37+
set { }
38+
}
39+
40+
// CHECK-LABEL: sil hidden [ossa] @$s23assign_or_init_lowering25TestMemberwiseWithOverlapV1a4pair1cACSi_Si_SitSitcfC : $@convention(method) (Int, Int, Int, Int, @thin TestMemberwiseWithOverlap.Type) -> TestMemberwiseWithOverlap
41+
//
42+
// CHECK: [[SELF:%.*]] = alloc_stack $TestMemberwiseWithOverlap
43+
//
44+
// CHECK: [[A_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._a
45+
// CHECK: [[A_INIT:%.*]] = function_ref @$s23assign_or_init_lowering25TestMemberwiseWithOverlapV1aSivi : $@convention(thin) (Int) -> @out Int
46+
// CHECK-NEXT: {{.*}} = apply [[A_INIT]]([[A_REF]], %0) : $@convention(thin) (Int) -> @out Int
47+
//
48+
// CHECK: [[A_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._a
49+
// CHECK-NEXT: destroy_addr [[A_REF]] : $*Int
50+
// CHECK-NEXT: [[B_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._b
51+
// CHECK: [[PAIR_INIT:%.*]] = function_ref @$s23assign_or_init_lowering25TestMemberwiseWithOverlapV4pairSi_Sitvi : $@convention(thin) (Int, Int) -> (@out Int, @out Int)
52+
// CHECK-NEXT: {{.*}} = apply [[PAIR_INIT]]([[A_REF]], [[B_REF]], %1, %2) : $@convention(thin) (Int, Int) -> (@out Int, @out Int)
53+
//
54+
// CHECK: [[B_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._b
55+
// CHECK-NEXT: destroy_addr [[B_REF]] : $*Int
56+
// CHECK-NEXT: [[C_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._c
57+
// CHECK-NEXT: [[A_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._a
58+
// CHECK: [[C_INIT:%.*]] = function_ref @$s23assign_or_init_lowering25TestMemberwiseWithOverlapV1cSivi : $@convention(thin) (Int, @inout Int) -> (@out Int, @out Int)
59+
// CHECK-NEXT: {{.*}} = apply [[C_INIT]]([[B_REF]], [[C_REF]], %3, [[A_REF]]) : $@convention(thin) (Int, @inout Int) -> (@out Int, @out Int)
60+
// CHECK-NEXT: [[RESULT:%.*]] = load [trivial] [[SELF]] : $*TestMemberwiseWithOverlap
61+
// CHECK-NEXT: dealloc_stack [[SELF]] : $*TestMemberwiseWithOverlap
62+
// CHECK-NEXT: return [[RESULT]] : $TestMemberwiseWithOverlap
63+
}
64+
65+
_ = TestMemberwiseWithOverlap(a: 0, pair: (1, 2), c: 3)
66+
567
struct Test1 {
668
var _a: Int
769
var _b: String

0 commit comments

Comments
 (0)