Merge pull request swiftlang#67982 from tshortli/back-deployed-keypaths

SILGen: Don't reference external property descriptors for @backDeployed properties

Author: tshortli

lib/SILGen/SILGen.h

@@ -351,6 +351,11 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor<SILGenModule> {
   /// Emits a thunk from an actor function to a potentially distributed call.
   void emitDistributedThunk(SILDeclRef thunk);
 
+  /// Returns true if the given declaration must be referenced through a
+  /// back deployment thunk in a context with the given resilience expansion.
+  bool requiresBackDeploymentThunk(ValueDecl *decl,
+                                   ResilienceExpansion expansion);
+
   /// Emits a thunk that calls either the original function if it is available
   /// or otherwise calls a fallback variant of the function that was emitted
   /// into the client module.

lib/SILGen/SILGenApply.cpp

@@ -280,31 +280,6 @@ static void convertOwnershipConventionsGivenParamInfos(
                   });
 }
 
-static bool shouldApplyBackDeploymentThunk(ValueDecl *decl, ASTContext &ctx,
-                                           ResilienceExpansion expansion) {
-  auto backDeployBeforeVersion = decl->getBackDeployedBeforeOSVersion(ctx);
-  if (!backDeployBeforeVersion)
-    return false;
-
-  // If the context of the application is inlinable then we must always call the
-  // back deployment thunk since we can't predict the deployment targets of
-  // other modules.
-  if (expansion != ResilienceExpansion::Maximal)
-    return true;
-
-  // In resilient function bodies skip calling the back deployment thunk when
-  // the deployment target is high enough that the ABI implementation of the
-  // back deployed function is guaranteed to be available.
-  auto deploymentAvailability = AvailabilityContext::forDeploymentTarget(ctx);
-  auto declAvailability =
-      AvailabilityContext(VersionRange::allGTE(*backDeployBeforeVersion));
-
-  if (deploymentAvailability.isContainedIn(declAvailability))
-    return false;
-
-  return true;
-}
-
 //===----------------------------------------------------------------------===//
 //                                   Callee
 //===----------------------------------------------------------------------===//
@@ -1155,7 +1130,6 @@ class SILGenApply : public Lowering::ExprVisitor<SILGenApply> {
 
   SILDeclRef getDeclRefForStaticDispatchApply(DeclRefExpr *e) {
     auto *afd = cast<AbstractFunctionDecl>(e->getDecl());
-    auto &ctx = SGF.getASTContext();
 
     // A call to a `distributed` function may need to go through a thunk.
     if (callSite && callSite->shouldApplyDistributedThunk()) {
@@ -1164,8 +1138,9 @@ class SILGenApply : public Lowering::ExprVisitor<SILGenApply> {
     }
 
     // A call to `@backDeployed` function may need to go through a thunk.
-    if (shouldApplyBackDeploymentThunk(afd, ctx,
-                                       SGF.F.getResilienceExpansion())) {
+
+    if (SGF.SGM.requiresBackDeploymentThunk(afd,
+                                            SGF.F.getResilienceExpansion())) {
       return SILDeclRef(afd).asBackDeploymentKind(
           SILDeclRef::BackDeploymentKind::Thunk);
     }
@@ -6717,7 +6692,7 @@ SILDeclRef SILGenModule::getAccessorDeclRef(AccessorDecl *accessor,
                                             ResilienceExpansion expansion) {
   auto declRef = SILDeclRef(accessor, SILDeclRef::Kind::Func);
 
-  if (shouldApplyBackDeploymentThunk(accessor, getASTContext(), expansion))
+  if (requiresBackDeploymentThunk(accessor, expansion))
     return declRef.asBackDeploymentKind(SILDeclRef::BackDeploymentKind::Thunk);
 
   return declRef.asForeign(requiresForeignEntryPoint(accessor));

lib/SILGen/SILGenBackDeploy.cpp

@@ -168,6 +168,36 @@ static void emitBackDeployForwardApplyAndReturnOrThrow(
   SGF.B.createBranch(loc, SGF.ReturnDest.getBlock(), directResults);
 }
 
+bool SILGenModule::requiresBackDeploymentThunk(ValueDecl *decl,
+                                               ResilienceExpansion expansion) {
+  auto &ctx = getASTContext();
+  auto backDeployBeforeVersion = decl->getBackDeployedBeforeOSVersion(ctx);
+  if (!backDeployBeforeVersion)
+    return false;
+
+  switch (expansion) {
+  case ResilienceExpansion::Minimal:
+    // In a minimal resilience expansion we must always call the back deployment
+    // thunk since we can't predict the deployment targets of the modules that
+    // might inline the call.
+    return true;
+  case ResilienceExpansion::Maximal:
+    // FIXME: We can skip thunking if we're in the same module.
+    break;
+  }
+
+  // Use of a back deployment thunk is unnecessary if the deployment target is
+  // high enough that the ABI implementation of the back deployed declaration is
+  // guaranteed to be available.
+  auto deploymentAvailability = AvailabilityContext::forDeploymentTarget(ctx);
+  auto declAvailability =
+      AvailabilityContext(VersionRange::allGTE(*backDeployBeforeVersion));
+  if (deploymentAvailability.isContainedIn(declAvailability))
+    return false;
+
+  return true;
+}
+
 void SILGenFunction::emitBackDeploymentThunk(SILDeclRef thunk) {
   // Generate code equivalent to:
   //

lib/SILGen/SILGenExpr.cpp

@@ -3821,6 +3821,12 @@ SILGenModule::emitKeyPathComponentForDecl(SILLocation loc,
       return false;
     }
 
+    // Back deployed properties have the same restrictions as
+    // always-emit-into-client properties.
+    if (requiresBackDeploymentThunk(baseDecl, expansion)) {
+      return false;
+    }
+
     // Properties that only dispatch via ObjC lookup do not have nor
     // need property descriptors, since the selector identifies the
     // storage.

test/SILGen/back_deployed_attr_accessor.swift

@@ -45,4 +45,11 @@ func caller(_ s: TopLevelStruct) {
   // -- Verify the thunk is called
   // CHECK: {{%.*}} = function_ref @$s11back_deploy14TopLevelStructV8propertyACvgTwb : $@convention(method) (TopLevelStruct) -> TopLevelStruct
   _ = s.property
+
+  // -- Verify key path
+  // CHECK: {{%.*}} = keypath $KeyPath<TopLevelStruct, TopLevelStruct>, (root $TopLevelStruct; gettable_property $TopLevelStruct,  id @$s11back_deploy14TopLevelStructV8propertyACvg : $@convention(method) (TopLevelStruct) -> TopLevelStruct, getter @$s11back_deploy14TopLevelStructV8propertyACvpACTK : $@convention(thin) (@in_guaranteed TopLevelStruct) -> @out TopLevelStruct)
+  _ = \TopLevelStruct.property
 }
+
+// CHECK-LABEL: sil shared [thunk] [ossa] @$s11back_deploy14TopLevelStructV8propertyACvpACTK : $@convention(thin) (@in_guaranteed TopLevelStruct) -> @out TopLevelStruct
+// CHECK: function_ref @$s11back_deploy14TopLevelStructV8propertyACvgTwb

test/attr/attr_backDeployed_evolution.swift

@@ -149,6 +149,7 @@ do {
 
   let empty = IntArray.empty
   precondition(empty.values == [])
+  precondition(empty[keyPath: \.values] == [])
 
   var array = IntArray([5])
 
@@ -177,6 +178,7 @@ do {
 do {
   let defaulted = ReferenceIntArray()
   precondition(defaulted.values == [])
+  precondition(defaulted[keyPath: \.values] == [])
 
   let empty = ReferenceIntArray.empty
   precondition(empty.values == [])