Judge the need for an external key path reference by the overridden base.

We identify overrides using the same key path descriptor as the base, but an overridden base
may not have a property descriptor if it's @objc or fragile. Fixes rdar://problem/51479334.

Author: jckarter

lib/SILGen/SILGenExpr.cpp

@@ -3419,23 +3419,32 @@ SILGenModule::emitKeyPathComponentForDecl(SILLocation loc,
                                 ArrayRef<ProtocolConformanceRef> indexHashables,
                                 CanType baseTy,
                                 bool forPropertyDescriptor) {
+  auto baseDecl = storage;
+
+  // ABI-compatible overrides do not have property descriptors, so we need
+  // to reference the overridden declaration instead.
+  if (isa<ClassDecl>(baseDecl->getDeclContext())) {
+    while (!baseDecl->isValidKeyPathComponent())
+      baseDecl = baseDecl->getOverriddenDecl();
+  }
+
   /// Returns true if a key path component for the given property or
   /// subscript should be externally referenced.
   auto shouldUseExternalKeyPathComponent = [&]() -> bool {
     return (!forPropertyDescriptor &&
-            (storage->getModuleContext() != SwiftModule ||
-             storage->isResilient(SwiftModule, expansion)) &&
+            (baseDecl->getModuleContext() != SwiftModule ||
+             baseDecl->isResilient(SwiftModule, expansion)) &&
             // Protocol requirements don't have nor need property descriptors.
-            !isa<ProtocolDecl>(storage->getDeclContext()) &&
+            !isa<ProtocolDecl>(baseDecl->getDeclContext()) &&
             // Properties that only dispatch via ObjC lookup do not have nor
             // need property descriptors, since the selector identifies the
             // storage.
             // Properties that are not public don't need property descriptors
             // either.
-            (!storage->hasAnyAccessors() ||
-             (!getAccessorDeclRef(getRepresentativeAccessorForKeyPath(storage))
+            (!baseDecl->hasAnyAccessors() ||
+             (!getAccessorDeclRef(getRepresentativeAccessorForKeyPath(baseDecl))
                    .isForeign &&
-              getAccessorDeclRef(getRepresentativeAccessorForKeyPath(storage))
+              getAccessorDeclRef(getRepresentativeAccessorForKeyPath(baseDecl))
                       .getLinkage(ForDefinition) <= SILLinkage::PublicNonABI)));
   };
 
@@ -3465,10 +3474,7 @@ SILGenModule::emitKeyPathComponentForDecl(SILLocation loc,
 
     // ABI-compatible overrides do not have property descriptors, so we need
     // to reference the overridden declaration instead.
-    auto *baseDecl = externalDecl;
-    if (isa<ClassDecl>(baseDecl->getDeclContext())) {
-      while (!baseDecl->isValidKeyPathComponent())
-        baseDecl = baseDecl->getOverriddenDecl();
+    if (baseDecl != externalDecl) {
       externalSubs = SubstitutionMap::getOverrideSubstitutions(baseDecl,
                                                                externalDecl,
                                                                externalSubs);

test/SILGen/keypaths_objc.swift

@@ -102,3 +102,14 @@ func sharedCProperty() {
   // CHECK-NOT: external #c_union.some_field
   let dataKeyPath: WritableKeyPath<c_union, some_struct>? = \c_union.some_field
 }
+
+class OverrideFrameworkObjCProperty: A {
+  override var counter: Int32 {
+    get { return 0 }
+    set { }
+  }
+}
+
+func overrideFrameworkObjCProperty() {
+  let _ = \OverrideFrameworkObjCProperty.counter
+}