Merge pull request swiftlang#30053 from slavapestov/keypath-component-resilient-stored-generic

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

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: )