Skip to content

Commit cffc3fd

Browse files
xedinxedin
committed
[Sema/SILGen] InitAccessors: Don't synthesize memberwise init if initializes intersect
If some property is initializable by one than one init accessor let's not sythesize a memberwise initializer in that case because it's ambiguous what is the best init accessor to use.
1 parent 4f59538 commit cffc3fd

File tree

5 files changed

+36
-229
lines changed

5 files changed

+36
-229
lines changed

Diff for: ‎lib/SILGen/SILGenConstructor.cpp

+13-32
Original file line numberDiff line numberDiff line change
@@ -275,22 +275,14 @@ static RValue maybeEmitPropertyWrapperInitFromValue(
275275
static void
276276
emitApplyOfInitAccessor(SILGenFunction &SGF, SILLocation loc,
277277
AccessorDecl *accessor, SILValue selfValue,
278-
SILType selfTy, RValue &&initialValue,
279-
llvm::SmallPtrSetImpl<VarDecl *> &initializedFields) {
278+
SILType selfTy, RValue &&initialValue) {
280279
SmallVector<SILValue> arguments;
281280

282281
auto emitFieldReference = [&](VarDecl *field, bool forInit = false) {
283282
auto fieldTy =
284283
selfTy.getFieldType(field, SGF.SGM.M, SGF.getTypeExpansionContext());
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;
284+
return SGF.B.createStructElementAddr(loc, selfValue, field,
285+
fieldTy.getAddressType());
294286
};
295287

296288
// First, let's emit all of the indirect results.
@@ -411,35 +403,24 @@ static void emitImplicitValueConstructor(SILGenFunction &SGF,
411403
// Tracks all the init accessors we have emitted
412404
// because they can initialize more than one property.
413405
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;
417-
418406
auto elti = elements.begin(), eltEnd = elements.end();
419407
for (VarDecl *field : decl->getStoredProperties()) {
420408

421409
// Handle situations where this stored propery is initialized
422410
// via a call to an init accessor on some other property.
423-
if (initializedViaAccessor.count(field)) {
424-
for (auto iter = initializedViaAccessor.find(field);
425-
iter != initializedViaAccessor.end(); ++iter) {
426-
auto *initProperty = iter->second;
427-
auto *initAccessor = initProperty->getAccessor(AccessorKind::Init);
428-
429-
if (!emittedInitAccessors.insert(initAccessor).second)
430-
continue;
411+
if (initializedViaAccessor.count(field) == 1) {
412+
auto *initProperty = initializedViaAccessor.find(field)->second;
413+
auto *initAccessor = initProperty->getAccessor(AccessorKind::Init);
431414

432-
assert(elti != eltEnd &&
433-
"number of args does not match number of fields");
415+
if (!emittedInitAccessors.insert(initAccessor).second)
416+
continue;
434417

435-
emitApplyOfInitAccessor(SGF, Loc, initAccessor, resultSlot, selfTy,
436-
std::move(*elti),
437-
fieldsInitializedViaAccessor);
438-
++elti;
439-
}
418+
assert(elti != eltEnd &&
419+
"number of args does not match number of fields");
440420

441-
// After all init accessors are emitted let's move on to the next
442-
// property.
421+
emitApplyOfInitAccessor(SGF, Loc, initAccessor, resultSlot, selfTy,
422+
std::move(*elti));
423+
++elti;
443424
continue;
444425
}
445426

Diff for: ‎lib/Sema/CodeSynthesis.cpp

+16-9
Original file line numberDiff line numberDiff line change
@@ -320,14 +320,10 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl,
320320
continue;
321321

322322
// Check whether this property could be initialized via init accessor.
323-
if (initializedViaAccessor.count(var)) {
324-
for (auto iter = initializedViaAccessor.find(var);
325-
iter != initializedViaAccessor.end(); ++iter) {
326-
auto *initializerProperty = iter->second;
327-
328-
if (usedInitProperties.insert(initializerProperty).second)
329-
createParameter(initializerProperty);
330-
}
323+
if (initializedViaAccessor.count(var) == 1) {
324+
auto *initializerProperty = initializedViaAccessor.find(var)->second;
325+
if (usedInitProperties.insert(initializerProperty).second)
326+
createParameter(initializerProperty);
331327
} else {
332328
createParameter(var);
333329
}
@@ -1337,9 +1333,20 @@ HasMemberwiseInitRequest::evaluate(Evaluator &evaluator,
13371333
if (initializedViaAccessor.empty())
13381334
return true;
13391335

1340-
if (!initializedViaAccessor.count(var)) {
1336+
switch (initializedViaAccessor.count(var)) {
1337+
// Not covered by an init accessor.
1338+
case 0:
13411339
initializedProperties.insert(var);
13421340
continue;
1341+
1342+
// Covered by a single init accessor.
1343+
case 1:
1344+
break;
1345+
1346+
// Covered by one than one init accessor which means that we
1347+
// cannot synthesize memberwise initializer.
1348+
default:
1349+
return false;
13431350
}
13441351

13451352
// Check whether use of init accessors results in access to uninitialized

Diff for: ‎test/Interpreter/init_accessors.swift

-122
Original file line numberDiff line numberDiff line change
@@ -332,125 +332,3 @@ 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,68 +2,6 @@
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-
675
struct Test1 {
686
var _a: Int
697
var _b: String

Diff for: ‎test/decl/var/init_accessors.swift

+7-4
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ func test_invalid_references() {
243243
}
244244
}
245245

246-
func test_memberwise_with_overlaps() {
246+
func test_memberwise_with_overlaps_dont_synthesize_inits() {
247247
struct Test1<T, U> {
248248
var _a: T
249249
var _b: Int
@@ -270,7 +270,8 @@ func test_memberwise_with_overlaps() {
270270
var c: U
271271
}
272272

273-
_ = Test1(a: "a", pair: ("b", 1), c: [3.0]) // Ok
273+
_ = Test1<String, [Double]>(a: "a", pair: ("b", 1), c: [3.0])
274+
// expected-error@-1 {{'Test1<String, [Double]>' cannot be constructed because it has no accessible initializers}}
274275

275276
struct Test2<T, U> {
276277
var _a: T
@@ -307,7 +308,8 @@ func test_memberwise_with_overlaps() {
307308
}
308309
}
309310

310-
_ = Test2(a: "a", pair: ("c", 2), b: 0) // Ok
311+
_ = Test2<String, Int>(a: "a", pair: ("c", 2), b: 0)
312+
// expected-error@-1 {{'Test2<String, Int>' cannot be constructed because it has no accessible initializers}}
311313

312314
struct Test3<T, U> {
313315
var _a: T
@@ -354,7 +356,8 @@ func test_memberwise_with_overlaps() {
354356
}
355357
}
356358

357-
_ = Test3(a: "a", triple: ("b", 2, [1.0, 2.0]), b: 0, c: [1.0]) // Ok
359+
_ = Test3<String, [Double]>(a: "a", triple: ("b", 2, [1.0, 2.0]), b: 0, c: [1.0])
360+
// expected-error@-1 {{'Test3<String, [Double]>' cannot be constructed because it has no accessible initializers}}
358361
}
359362

360363
func test_invalid_memberwise() {

0 commit comments

Comments
 (0)