In Swift 3/4 mode, continue treating 'lazy override' as an override (#13335)

Follow-up for 7c707ce. Without this, the declaration would be
accepted, but any uses of the overridden property would be treated as
ambiguous because the property wouldn't really be marked as an
override.

rdar://problem/35900345

Author: jrose-apple

lib/Sema/CodeSynthesis.cpp

@@ -172,6 +172,10 @@ static FuncDecl *createGetterPrototype(AbstractStorageDecl *storage,
   if (storage->isStatic())
     getter->setStatic();
 
+  if (auto *overridden = storage->getOverriddenDecl())
+    if (auto *overriddenAccessor = overridden->getGetter())
+      getter->setOverriddenDecl(overriddenAccessor);
+
   return getter;
 }
 
@@ -219,6 +223,14 @@ static FuncDecl *createSetterPrototype(AbstractStorageDecl *storage,
   if (isStatic)
     setter->setStatic();
 
+  if (auto *overridden = storage->getOverriddenDecl()) {
+    auto *overriddenAccessor = overridden->getSetter();
+    if (overriddenAccessor &&
+        overridden->isSetterAccessibleFrom(storage->getDeclContext())) {
+      setter->setOverriddenDecl(overriddenAccessor);
+    }
+  }
+
   return setter;
 }
 
@@ -351,7 +363,15 @@ static FuncDecl *createMaterializeForSetPrototype(AbstractStorageDecl *storage,
   // materializeForSet is final if the storage is.
   if (storage->isFinal())
     makeFinal(ctx, materializeForSet);
-  
+
+  if (auto *overridden = storage->getOverriddenDecl()) {
+    auto *overriddenAccessor = overridden->getMaterializeForSetFunc();
+    if (overriddenAccessor && !overriddenAccessor->hasForcedStaticDispatch() &&
+        overridden->isSetterAccessibleFrom(storage->getDeclContext())) {
+      materializeForSet->setOverriddenDecl(overriddenAccessor);
+    }
+  }
+
   // If the storage is dynamic or ObjC-native, we can't add a dynamically-
   // dispatched method entry for materializeForSet, so force it to be
   // statically dispatched. ("final" would be inappropriate because the
@@ -709,11 +729,6 @@ static void synthesizeTrivialGetter(FuncDecl *getter,
   SourceLoc loc = storage->getLoc();
   getter->setBody(BraceStmt::create(ctx, loc, returnStmt, loc, true));
 
-  // Record the getter as an override, which can happen with addressors.
-  if (auto *baseASD = storage->getOverriddenDecl())
-    if (baseASD->isAccessibleFrom(storage->getDeclContext()))
-      getter->setOverriddenDecl(baseASD->getGetter());
-
   // Register the accessor as an external decl if the storage was imported.
   if (needsToBeRegisteredAsExternalDecl(storage))
     TC.Context.addExternalDecl(getter);
@@ -734,15 +749,6 @@ static void synthesizeTrivialSetter(FuncDecl *setter,
                                             setterBody, TC);
   setter->setBody(BraceStmt::create(ctx, loc, setterBody, loc, true));
 
-  // Record the setter as an override, which can happen with addressors.
-  if (auto *baseASD = storage->getOverriddenDecl()) {
-    auto *baseSetter = baseASD->getSetter();
-    if (baseSetter != nullptr &&
-        baseASD->isSetterAccessibleFrom(storage->getDeclContext())) {
-      setter->setOverriddenDecl(baseSetter);
-    }
-  }
-
   // Register the accessor as an external decl if the storage was imported.
   if (needsToBeRegisteredAsExternalDecl(storage))
     TC.Context.addExternalDecl(setter);
@@ -848,21 +854,6 @@ 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 &&
-        !baseMFS->hasForcedStaticDispatch() &&
-        baseASD->isSetterAccessibleFrom(storage->getDeclContext())) {
-      materializeForSet->setOverriddenDecl(baseMFS);
-    }
-  }
-
   return materializeForSet;
 }
 

lib/Sema/TypeCheckDecl.cpp

@@ -6627,18 +6627,22 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
 
       // Make sure that the overriding property doesn't have storage.
       if (overrideASD->hasStorage() && !overrideASD->hasObservers()) {
-        auto diagID = diag::override_with_stored_property;
+        bool downgradeToWarning = false;
         if (!TC.Context.isSwiftVersionAtLeast(5) &&
             overrideASD->getAttrs().hasAttribute<LazyAttr>()) {
           // Swift 4.0 had a bug where lazy properties were considered
           // computed by the time of this check. Downgrade this diagnostic to
           // a warning.
-          diagID = diag::override_with_stored_property_warn;
+          downgradeToWarning = true;
         }
+        auto diagID = downgradeToWarning ?
+            diag::override_with_stored_property_warn :
+            diag::override_with_stored_property;
         TC.diagnose(overrideASD, diagID,
                     overrideASD->getBaseName().getIdentifier());
         TC.diagnose(baseASD, diag::property_override_here);
-        return true;
+        if (!downgradeToWarning)
+          return true;
       }
 
       // Make sure that an observing property isn't observing something

test/Compatibility/attr_override_lazy.swift

@@ -0,0 +1,25 @@
+// RUN: %target-swift-frontend -swift-version 4 -emit-silgen %s | %FileCheck %s
+
+class Base {
+  var foo: Int { return 0 }
+  var bar: Int = 0
+}
+
+class Sub : Base {
+  lazy override var foo: Int = 1
+  lazy override var bar: Int = 1
+  func test() -> Int {
+    // CHECK-LABEL: sil {{.*}}@_T018attr_override_lazy3SubC4testSiyF
+    // CHECK: class_method %0 : $Sub, #Sub.foo!getter.1
+    // CHECK: class_method %0 : $Sub, #Sub.bar!getter.1
+    // CHECK: // end sil function '_T018attr_override_lazy3SubC4testSiyF'
+    return foo + bar // no ambiguity error here
+  }
+}
+
+// CHECK-LABEL: sil_vtable Sub {
+// CHECK: #Base.foo!getter.1: (Base) -> () -> Int : {{.*}} // Sub.foo.getter
+// CHECK: #Base.bar!getter.1: (Base) -> () -> Int : {{.*}} // Sub.bar.getter
+// CHECK: #Base.bar!setter.1: (Base) -> (Int) -> () : {{.*}} // Sub.bar.setter
+// CHECK: #Base.bar!materializeForSet.1: (Base) -> {{.*}} : {{.*}} // Sub.bar.materializeForSet
+// CHECK: }