SILGen: Work around for stored property keypath components not supporting generic resilient classes

A keypath component for a stored property can take one of several forms:

- The property offset is known to be constant at compile-time.

  This is used in the simplest cases for classes and structs.

- The property offset is not constant, but can be loaded from a global.

  This is used for classes that require runtime resilient layout, but where
  the offsets do not depend on the generic context.

- The property offset is not constant, and must be loaded from metadata.

  This is the case where the offset depends on the generic context. Here,
  we were only set up to load it from a fixed offset in the metadata.
  This works for generic structs, or generic classes where the superclass
  chain does not cross a resilience boundary.

  However, if a resilience boundary is crossed, the offset of the field
  offset in the metadata must itself be obtained at runtime by adding a
  constant to a value loaded from a global. This case is not supported by
  the current keypath ABI due to an oversight.

I filed <rdar://problem/59777983> to track extending the ABI to handle
this more elegantly in the future.

Fixes <rdar://problem/59617119>.

Author: slavapestov

lib/IRGen/GenKeyPath.cpp

@@ -794,7 +794,7 @@ emitKeyPathComponent(IRGenModule &IGM,
 
       switch (getClassFieldAccess(IGM, loweredBaseContextTy, property)) {
       case FieldAccess::ConstantDirect: {
-        // Known constant fixed offset.
+        // Known compile-time constant field offset.
         auto offset = tryEmitConstantClassFragilePhysicalMemberOffset(
             IGM, loweredClassTy, property);
         assert(offset && "no constant offset for ConstantDirect field?!");
@@ -804,6 +804,9 @@ emitKeyPathComponent(IRGenModule &IGM,
       case FieldAccess::NonConstantDirect: {
         // A constant offset that's determined at class realization time.
         // We have to load the offset from a global ivar.
+        //
+        // This means the field offset is constant at runtime, but is not known
+        // at compile time.
         auto header = KeyPathComponentHeader
           ::forClassComponentWithUnresolvedIndirectOffset(property->isLet());
         fields.addInt32(header.getData());
@@ -814,10 +817,15 @@ emitKeyPathComponent(IRGenModule &IGM,
       }
       case FieldAccess::ConstantIndirect: {
         // An offset that depends on the instance's generic parameterization,
-        // but whose field offset is at a known vtable offset.
+        // but whose field offset is at a known metadata offset.
         auto header = KeyPathComponentHeader
           ::forClassComponentWithUnresolvedFieldOffset(property->isLet());
         fields.addInt32(header.getData());
+
+        // FIXME: This doesn't support classes with resilient ancestry, because
+        // the offset into the metadata is itself not constant.
+        //
+        // SILGen emits the descriptor as a computed property in this case.
         auto fieldOffset = getClassFieldOffsetOffset(
             IGM, loweredClassTy.getClassOrBoundGenericClass(), property);
         fields.addInt32(fieldOffset.getValue());

lib/SILGen/SILGen.cpp

@@ -1353,6 +1353,24 @@ SILGenModule::canStorageUseStoredKeyPathComponent(AbstractStorageDecl *decl,
     // unowned properties.
     if (decl->getInterfaceType()->is<ReferenceStorageType>())
       return false;
+
+    // If the field offset depends on the generic instantiation, we have to
+    // load it from metadata when instantiating the keypath component.
+    //
+    // However the metadata offset itself will not be fixed if the superclass
+    // is resilient. Fall back to treating the property as computed in this
+    // case.
+    //
+    // See the call to getClassFieldOffsetOffset() inside
+    // emitKeyPathComponent().
+    if (auto *parentClass = dyn_cast<ClassDecl>(decl->getDeclContext())) {
+      if (parentClass->isGeneric()) {
+        auto ancestry = parentClass->checkAncestry();
+        if (ancestry.contains(AncestryFlags::ResilientOther))
+          return false;
+      }
+    }
+
     // If the stored value would need to be reabstracted in fully opaque
     // context, then we have to treat the component as computed.
     auto componentObjTy = decl->getValueInterfaceType();

test/SILGen/keypaths_resilient_generic.swift

@@ -0,0 +1,30 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -enable-library-evolution -emit-module-path=%t/resilient_struct.swiftmodule -module-name=resilient_struct %S/../Inputs/resilient_struct.swift
+// RUN: %target-swift-frontend -emit-module -enable-library-evolution -emit-module-path=%t/resilient_class.swiftmodule -module-name=resilient_class -I %t %S/../Inputs/resilient_class.swift
+
+// RUN: %target-swift-emit-silgen %s -I %t -enable-library-evolution | %FileCheck %s
+
+import resilient_class
+
+open class MySubclass<T> : ResilientOutsideParent {
+  public final var storedProperty: T? = nil
+}
+
+open class ConcreteSubclass : MySubclass<Int> {
+  public final var anotherStoredProperty: Int? = nil
+}
+
+// CHECK-LABEL: sil shared [thunk] [ossa] @$s26keypaths_resilient_generic10MySubclassC14storedPropertyxSgvplACyxGTK : $@convention(thin) <T> (@in_guaranteed MySubclass<T>) -> @out Optional<T> {
+
+// CHECK-LABEL: sil shared [thunk] [ossa] @$s26keypaths_resilient_generic10MySubclassC14storedPropertyxSgvplACyxGTk : $@convention(thin) <T> (@in_guaranteed Optional<T>, @in_guaranteed MySubclass<T>) -> () {
+
+// CHECK:      sil_property #MySubclass.storedProperty<τ_0_0> (
+// CHECK-SAME:   settable_property $Optional<τ_0_0>,
+// CHECK-SAME:   id ##MySubclass.storedProperty,
+// CHECK-SAME:   getter @$s26keypaths_resilient_generic10MySubclassC14storedPropertyxSgvplACyxGTK : $@convention(thin) <τ_0_0> (@in_guaranteed MySubclass<τ_0_0>) -> @out Optional<τ_0_0>,
+// CHECK-SAME:   setter @$s26keypaths_resilient_generic10MySubclassC14storedPropertyxSgvplACyxGTk : $@convention(thin) <τ_0_0> (@in_guaranteed Optional<τ_0_0>, @in_guaranteed MySubclass<τ_0_0>) -> ()
+// CHECK-SAME: )
+
+// CHECK:      sil_property #ConcreteSubclass.anotherStoredProperty (
+// CHECK-SAME:   stored_property #ConcreteSubclass.anotherStoredProperty : $Optional<Int>
+// CHECK-SAME: )