From 293227aaa0f0bce951199a1698effbb7bd7f939a Mon Sep 17 00:00:00 2001
From: Slava Pestov <spestov@apple.com>
Date: Thu, 2 Feb 2017 17:25:32 -0800
Subject: [PATCH] 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>.
---
 lib/Sema/CodeSynthesis.cpp          | 14 ++++++++++++++
 test/Interpreter/classes.swift      | 26 ++++++++++++++++++++++++++
 test/SILGen/materializeForSet.swift | 24 ++++++++++++++++++++++++
 3 files changed, 64 insertions(+)

diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp
index fcc284b2f5f49..9ff6f6564c55e 100644
--- a/lib/Sema/CodeSynthesis.cpp
+++ b/lib/Sema/CodeSynthesis.cpp
@@ -814,6 +814,20 @@ static FuncDecl *addMaterializeForSet(AbstractStorageDecl *storage,
                              storage->getSetter());
   storage->setMaterializeForSetFunc(materializeForSet);
 
+  // Make sure we record the override.
+  //
+  // FIXME: Instead, we should just not call checkOverrides() on
+  // storage until all accessors are in place.
+  if (auto *baseASD = storage->getOverriddenDecl()) {
+    // If the base storage has a private setter, we're not overriding
+    // materializeForSet either.
+    auto *baseMFS = baseASD->getMaterializeForSetFunc();
+    if (baseMFS != nullptr &&
+        baseASD->isSetterAccessibleFrom(storage->getDeclContext())) {
+      materializeForSet->setOverriddenDecl(baseMFS);
+    }
+  }
+
   return materializeForSet;
 }
 
diff --git a/test/Interpreter/classes.swift b/test/Interpreter/classes.swift
index 81c86362c432d..c94522f3d51cc 100644
--- a/test/Interpreter/classes.swift
+++ b/test/Interpreter/classes.swift
@@ -207,3 +207,29 @@ func makeOne<T : Makeable>(_: T.Type) -> T {
 }
 
 makeOne(Child.self).doSomething() // CHECK: Heaven!
+
+// https://bugs.swift.org/browse/SR-3840
+
+class BaseProperty {
+  var value: Int {
+    get { fatalError() }
+    set { fatalError() }
+  }
+
+  func increment() -> Self {
+    value += 1
+    return self
+  }
+}
+
+class DerivedProperty : BaseProperty {
+  override var value: Int {
+    get { return _value }
+    set { _value = newValue }
+  }
+
+  var _value: Int = 0
+}
+
+// CHECK: 1
+print(DerivedProperty().increment().value)
diff --git a/test/SILGen/materializeForSet.swift b/test/SILGen/materializeForSet.swift
index 6ffc4dab5e4a4..15879ed537058 100644
--- a/test/SILGen/materializeForSet.swift
+++ b/test/SILGen/materializeForSet.swift
@@ -385,6 +385,21 @@ func inoutAccessOfStaticProperty<T : Beverage>(_ t: T.Type) {
   increment(&t.abv)
 }
 
+// Test for materializeForSet vs overriden computed property of classes.
+class BaseForOverride {
+  var valueStored: Int
+  var valueComputed: Int { get { } set { } }
+
+  init(valueStored: Int) {
+    self.valueStored = valueStored
+  }
+}
+
+class DerivedForOverride : BaseForOverride {
+  override var valueStored: Int { get { } set { } }
+  override var valueComputed: Int { get { } set { } }
+}
+
 // Test for materializeForSet vs static properties of classes.
 
 class ReferenceBeer {
@@ -514,6 +529,15 @@ func testMaterializedSetter() {
   f.computed = f.computed
 }
 
+// CHECK-LABEL: sil_vtable DerivedForOverride {
+// CHECK:   #BaseForOverride.valueComputed!getter.1: (BaseForOverride) -> () -> Int : _T017materializeForSet07DerivedB8OverrideC13valueComputedSifg
+// CHECK:   #BaseForOverride.valueComputed!setter.1: (BaseForOverride) -> (Int) -> () : _T017materializeForSet07DerivedB8OverrideC13valueComputedSifs
+// CHECK:   #BaseForOverride.valueComputed!materializeForSet.1: (BaseForOverride) -> (Builtin.RawPointer, inout Builtin.UnsafeValueBuffer) -> (Builtin.RawPointer, Builtin.RawPointer?) : _T017materializeForSet07DerivedB8OverrideC13valueComputedSifm
+// CHECK:   #BaseForOverride.valueStored!getter.1: (BaseForOverride) -> () -> Int : _T017materializeForSet07DerivedB8OverrideC11valueStoredSifg
+// CHECK:   #BaseForOverride.valueStored!setter.1: (BaseForOverride) -> (Int) -> () : _T017materializeForSet07DerivedB8OverrideC11valueStoredSifs
+// CHECK:   #BaseForOverride.valueStored!materializeForSet.1: (BaseForOverride) -> (Builtin.RawPointer, inout Builtin.UnsafeValueBuffer) -> (Builtin.RawPointer, Builtin.RawPointer?) : _T017materializeForSet07DerivedB8OverrideC11valueStoredSifm
+// CHECK: }
+
 // CHECK-LABEL: sil_witness_table hidden Bill: Totalled module materializeForSet {
 // CHECK:   method #Totalled.total!getter.1: {{.*}} : @_T017materializeForSet4BillVAA8TotalledAaaDP5totalSifgTW
 // CHECK:   method #Totalled.total!setter.1: {{.*}} : @_T017materializeForSet4BillVAA8TotalledAaaDP5totalSifsTW