Merge pull request #25645 from jckarter/external-keypath-override-5.1

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

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
+}