Skip to content

Commit 293227a

Browse files
committed
Sema: Use dynamic dispatch for inout access of properties in classes
The problem is that the derived property's materializeForSet was being synthesized after recordOverride() was called, so the new accessor never got setOverridenDecl() called on it. As a result, SIL didn't know that the derived materializeForSet should replace the vtable entry for the base class materializeForSet. The more fundamental problem here is that even after some recent cleanups, accessors are still sometimes type checked before the AbstractStorageDecla and sometimes after. So things like inferring final, dynamic and overrides have to be duplicated in multiple places. Fixes <https://bugs.swift.org/browse/SR-3840> and <rdar://problem/30336146>.
1 parent 2a25a8c commit 293227a

File tree

3 files changed

+64
-0
lines changed

3 files changed

+64
-0
lines changed

lib/Sema/CodeSynthesis.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -814,6 +814,20 @@ static FuncDecl *addMaterializeForSet(AbstractStorageDecl *storage,
814814
storage->getSetter());
815815
storage->setMaterializeForSetFunc(materializeForSet);
816816

817+
// Make sure we record the override.
818+
//
819+
// FIXME: Instead, we should just not call checkOverrides() on
820+
// storage until all accessors are in place.
821+
if (auto *baseASD = storage->getOverriddenDecl()) {
822+
// If the base storage has a private setter, we're not overriding
823+
// materializeForSet either.
824+
auto *baseMFS = baseASD->getMaterializeForSetFunc();
825+
if (baseMFS != nullptr &&
826+
baseASD->isSetterAccessibleFrom(storage->getDeclContext())) {
827+
materializeForSet->setOverriddenDecl(baseMFS);
828+
}
829+
}
830+
817831
return materializeForSet;
818832
}
819833

test/Interpreter/classes.swift

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,3 +207,29 @@ func makeOne<T : Makeable>(_: T.Type) -> T {
207207
}
208208

209209
makeOne(Child.self).doSomething() // CHECK: Heaven!
210+
211+
// https://bugs.swift.org/browse/SR-3840
212+
213+
class BaseProperty {
214+
var value: Int {
215+
get { fatalError() }
216+
set { fatalError() }
217+
}
218+
219+
func increment() -> Self {
220+
value += 1
221+
return self
222+
}
223+
}
224+
225+
class DerivedProperty : BaseProperty {
226+
override var value: Int {
227+
get { return _value }
228+
set { _value = newValue }
229+
}
230+
231+
var _value: Int = 0
232+
}
233+
234+
// CHECK: 1
235+
print(DerivedProperty().increment().value)

test/SILGen/materializeForSet.swift

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,21 @@ func inoutAccessOfStaticProperty<T : Beverage>(_ t: T.Type) {
385385
increment(&t.abv)
386386
}
387387

388+
// Test for materializeForSet vs overriden computed property of classes.
389+
class BaseForOverride {
390+
var valueStored: Int
391+
var valueComputed: Int { get { } set { } }
392+
393+
init(valueStored: Int) {
394+
self.valueStored = valueStored
395+
}
396+
}
397+
398+
class DerivedForOverride : BaseForOverride {
399+
override var valueStored: Int { get { } set { } }
400+
override var valueComputed: Int { get { } set { } }
401+
}
402+
388403
// Test for materializeForSet vs static properties of classes.
389404

390405
class ReferenceBeer {
@@ -514,6 +529,15 @@ func testMaterializedSetter() {
514529
f.computed = f.computed
515530
}
516531

532+
// CHECK-LABEL: sil_vtable DerivedForOverride {
533+
// CHECK: #BaseForOverride.valueComputed!getter.1: (BaseForOverride) -> () -> Int : _T017materializeForSet07DerivedB8OverrideC13valueComputedSifg
534+
// CHECK: #BaseForOverride.valueComputed!setter.1: (BaseForOverride) -> (Int) -> () : _T017materializeForSet07DerivedB8OverrideC13valueComputedSifs
535+
// CHECK: #BaseForOverride.valueComputed!materializeForSet.1: (BaseForOverride) -> (Builtin.RawPointer, inout Builtin.UnsafeValueBuffer) -> (Builtin.RawPointer, Builtin.RawPointer?) : _T017materializeForSet07DerivedB8OverrideC13valueComputedSifm
536+
// CHECK: #BaseForOverride.valueStored!getter.1: (BaseForOverride) -> () -> Int : _T017materializeForSet07DerivedB8OverrideC11valueStoredSifg
537+
// CHECK: #BaseForOverride.valueStored!setter.1: (BaseForOverride) -> (Int) -> () : _T017materializeForSet07DerivedB8OverrideC11valueStoredSifs
538+
// CHECK: #BaseForOverride.valueStored!materializeForSet.1: (BaseForOverride) -> (Builtin.RawPointer, inout Builtin.UnsafeValueBuffer) -> (Builtin.RawPointer, Builtin.RawPointer?) : _T017materializeForSet07DerivedB8OverrideC11valueStoredSifm
539+
// CHECK: }
540+
517541
// CHECK-LABEL: sil_witness_table hidden Bill: Totalled module materializeForSet {
518542
// CHECK: method #Totalled.total!getter.1: {{.*}} : @_T017materializeForSet4BillVAA8TotalledAaaDP5totalSifgTW
519543
// CHECK: method #Totalled.total!setter.1: {{.*}} : @_T017materializeForSet4BillVAA8TotalledAaaDP5totalSifsTW

0 commit comments

Comments
 (0)