Merge pull request swiftlang#38633 from nate-chandler/cherrypick/rdar79383990/release/5.5

nate-chandler · web-flow · commit 108cc64c3666 · 2021-08-10T09:54:57.000-07:00
[5.5] [SILGen] Used formal type when bridging completion handler arguments.
diff --git a/include/swift/SIL/AbstractionPattern.h b/include/swift/SIL/AbstractionPattern.h
@@ -1379,6 +1379,13 @@ class AbstractionPattern {
   /// Swift type.
   AbstractionPattern getObjCMethodAsyncCompletionHandlerType(
                                      CanType swiftCompletionHandlerType) const;
+
+  /// If this pattern refers to a foreign ObjC method that was imported as 
+  /// async, return the bridged-back-to-ObjC completion handler type.
+  CanType getObjCMethodAsyncCompletionHandlerForeignType(
+      ForeignAsyncConvention convention,
+      Lowering::TypeConverter &TC
+  ) const;
   
   void dump() const LLVM_ATTRIBUTE_USED;
   void print(raw_ostream &OS) const;
diff --git a/lib/SIL/IR/AbstractionPattern.cpp b/lib/SIL/IR/AbstractionPattern.cpp
@@ -661,6 +661,30 @@ AbstractionPattern::getObjCMethodAsyncCompletionHandlerType(
   }
 }
 
+
+CanType AbstractionPattern::getObjCMethodAsyncCompletionHandlerForeignType(
+    ForeignAsyncConvention convention,
+    Lowering::TypeConverter &TC
+) const {
+  auto nativeCHTy = convention.completionHandlerType();
+
+  // Use the abstraction pattern we're lowering against in order to lower
+  // the completion handler type, so we can preserve C/ObjC distinctions that
+  // normally get abstracted away by the importer.
+  auto completionHandlerNativeOrigTy = getObjCMethodAsyncCompletionHandlerType(nativeCHTy);
+  
+  // Bridge the Swift completion handler type back to its
+  // foreign representation.
+  auto foreignCHTy = TC.getLoweredBridgedType(completionHandlerNativeOrigTy,
+                                    nativeCHTy,
+                                    Bridgeability::Full,
+                                    SILFunctionTypeRepresentation::ObjCMethod,
+                                    TypeConverter::ForArgument)
+    ->getCanonicalType();
+
+  return foreignCHTy;
+}
+
 AbstractionPattern
 AbstractionPattern::getFunctionParamType(unsigned index) const {
   switch (getKind()) {
diff --git a/lib/SIL/IR/SILFunctionType.cpp b/lib/SIL/IR/SILFunctionType.cpp
@@ -1676,25 +1676,9 @@ class DestructureInputs {
         || NextOrigParamIndex != Foreign.Async->completionHandlerParamIndex())
       return false;
     
-    auto nativeCHTy = Foreign.Async->completionHandlerType();
-
-    // Use the abstraction pattern we're lowering against in order to lower
-    // the completion handler type, so we can preserve C/ObjC distinctions that
-    // normally get abstracted away by the importer.
-    auto completionHandlerNativeOrigTy = TopLevelOrigType
-      .getObjCMethodAsyncCompletionHandlerType(nativeCHTy);
-    
-    // Bridge the Swift completion handler type back to its
-    // foreign representation.
-    auto foreignCHTy = TC.getLoweredBridgedType(completionHandlerNativeOrigTy,
-                                      nativeCHTy,
-                                      Bridgeability::Full,
-                                      SILFunctionTypeRepresentation::ObjCMethod,
-                                      TypeConverter::ForArgument)
-      ->getCanonicalType();
-    
-    auto completionHandlerOrigTy = TopLevelOrigType
-      .getObjCMethodAsyncCompletionHandlerType(foreignCHTy);
+    CanType foreignCHTy = TopLevelOrigType
+      .getObjCMethodAsyncCompletionHandlerForeignType(Foreign.Async.getValue(), TC);
+    auto completionHandlerOrigTy = TopLevelOrigType.getObjCMethodAsyncCompletionHandlerType(foreignCHTy);
     auto completionHandlerTy = TC.getLoweredType(completionHandlerOrigTy,
                                                  foreignCHTy, expansion)
       .getASTType();
diff --git a/lib/SIL/IR/SILInstructions.cpp b/lib/SIL/IR/SILInstructions.cpp
@@ -2784,7 +2784,7 @@ SILType GetAsyncContinuationInstBase::getLoweredResumeType() const {
   auto formalType = getFormalResumeType();
   auto &M = getFunction()->getModule();
   auto c = getFunction()->getTypeExpansionContext();
-  return M.Types.getLoweredType(AbstractionPattern::getOpaque(), formalType, c);
+  return M.Types.getLoweredType(AbstractionPattern(formalType), formalType, c);
 }
 
 ReturnInst::ReturnInst(SILFunction &func, SILDebugLocation debugLoc,
diff --git a/lib/SILGen/Callee.h b/lib/SILGen/Callee.h
@@ -23,6 +23,7 @@ namespace Lowering {
 
 class CalleeTypeInfo {
 public:
+  Optional<AbstractionPattern> origFormalType;
   CanSILFunctionType substFnType;
   Optional<AbstractionPattern> origResultType;
   CanType substResultType;
@@ -45,17 +46,18 @@ class CalleeTypeInfo {
                  const Optional<ForeignAsyncConvention> &foreignAsync,
                  ImportAsMemberStatus foreignSelf,
                  Optional<SILFunctionTypeRepresentation> overrideRep = None)
-      : substFnType(substFnType), origResultType(origResultType),
-        substResultType(substResultType),
-        foreign{foreignError, foreignAsync, foreignSelf},
+      : origFormalType(llvm::None), substFnType(substFnType),
+        origResultType(origResultType),
+        substResultType(substResultType), foreign{foreignError, foreignAsync,
+                                                  foreignSelf},
         overrideRep(overrideRep) {}
 
   CalleeTypeInfo(CanSILFunctionType substFnType,
                  AbstractionPattern origResultType, CanType substResultType,
                  Optional<SILFunctionTypeRepresentation> overrideRep = None)
-      : substFnType(substFnType), origResultType(origResultType),
-        substResultType(substResultType), foreign(),
-        overrideRep(overrideRep) {}
+      : origFormalType(llvm::None), substFnType(substFnType),
+        origResultType(origResultType), substResultType(substResultType),
+        foreign(), overrideRep(overrideRep) {}
 
   SILFunctionTypeRepresentation getOverrideRep() const {
     return overrideRep.getValueOr(substFnType->getRepresentation());
diff --git a/lib/SILGen/ResultPlan.cpp b/lib/SILGen/ResultPlan.cpp
@@ -465,8 +465,9 @@ class ForeignAsyncInitializationPlan final : public ResultPlan {
   {
     // Allocate space to receive the resume value when the continuation is
     // resumed.
-    opaqueResumeType = SGF.getLoweredType(AbstractionPattern::getOpaque(),
-                                          calleeTypeInfo.substResultType);
+    opaqueResumeType =
+        SGF.getLoweredType(AbstractionPattern(calleeTypeInfo.substResultType),
+                           calleeTypeInfo.substResultType);
     resumeBuf = SGF.emitTemporaryAllocation(loc, opaqueResumeType);
   }
   
@@ -475,10 +476,11 @@ class ForeignAsyncInitializationPlan final : public ResultPlan {
                             SmallVectorImpl<SILValue> &outList) const override {
     // A foreign async function shouldn't have any indirect results.
   }
-  
+
   ManagedValue
-  emitForeignAsyncCompletionHandler(SILGenFunction &SGF, SILLocation loc)
-  override {
+  emitForeignAsyncCompletionHandler(SILGenFunction &SGF,
+                                    AbstractionPattern origFormalType,
+                                    SILLocation loc) override {
     // Get the current continuation for the task.
     bool throws = calleeTypeInfo.foreign.async
         ->completionHandlerErrorParamIndex().hasValue();
@@ -527,13 +529,12 @@ class ForeignAsyncInitializationPlan final : public ResultPlan {
     auto env = SGF.F.getGenericEnvironment();
     auto sig = env ? env->getGenericSignature()->getCanonicalSignature()
                    : CanGenericSignature();
-    SILFunction *impl = SGF.SGM
-      .getOrCreateForeignAsyncCompletionHandlerImplFunction(
-                  cast<SILFunctionType>(impFnTy->mapTypeOutOfContext()
-                                               ->getCanonicalType(sig)),
-                  continuationTy->mapTypeOutOfContext()->getCanonicalType(sig),
-                  sig,
-                  *calleeTypeInfo.foreign.async);
+    SILFunction *impl =
+        SGF.SGM.getOrCreateForeignAsyncCompletionHandlerImplFunction(
+            cast<SILFunctionType>(
+                impFnTy->mapTypeOutOfContext()->getCanonicalType(sig)),
+            continuationTy->mapTypeOutOfContext()->getCanonicalType(sig),
+            origFormalType, sig, *calleeTypeInfo.foreign.async);
     auto impRef = SGF.B.createFunctionRef(loc, impl);
     
     // Initialize the block object for the completion handler.
@@ -551,7 +552,7 @@ class ForeignAsyncInitializationPlan final : public ResultPlan {
     // _Block_copy-ing it.
     return ManagedValue::forUnmanaged(block);
   }
-  
+
   RValue finish(SILGenFunction &SGF, SILLocation loc, CanType substType,
                 ArrayRef<ManagedValue> &directResults) override {
     // There should be no direct results from the call.
diff --git a/lib/SILGen/ResultPlan.h b/lib/SILGen/ResultPlan.h
@@ -49,9 +49,9 @@ class ResultPlan {
   emitForeignErrorArgument(SILGenFunction &SGF, SILLocation loc) {
     return None;
   }
-  
-  virtual ManagedValue
-  emitForeignAsyncCompletionHandler(SILGenFunction &SGF, SILLocation loc) {
+
+  virtual ManagedValue emitForeignAsyncCompletionHandler(
+      SILGenFunction &SGF, AbstractionPattern origFormalType, SILLocation loc) {
     return {};
   }
 };
diff --git a/lib/SILGen/SILGen.h b/lib/SILGen/SILGen.h
@@ -185,10 +185,9 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor<SILGenModule> {
   /// implementation function for an ObjC API that was imported
   /// as `async` in Swift.
   SILFunction *getOrCreateForeignAsyncCompletionHandlerImplFunction(
-                                           CanSILFunctionType blockType,
-                                           CanType continuationTy,
-                                           CanGenericSignature sig,
-                                           ForeignAsyncConvention convention);
+      CanSILFunctionType blockType, CanType continuationTy,
+      AbstractionPattern origFormalType, CanGenericSignature sig,
+      ForeignAsyncConvention convention);
 
   /// Determine whether the given class has any instance variables that
   /// need to be destroyed.
diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp
@@ -3875,6 +3875,8 @@ RValue CallEmission::applyNormalCall(SGFContext C) {
   // Get the callee type information.
   auto calleeTypeInfo = callee.getTypeInfo(SGF);
 
+  calleeTypeInfo.origFormalType = origFormalType;
+
   // In C language modes, substitute the type of the AbstractionPattern
   // so that we won't see type parameters down when we try to form bridging
   // conversions.
@@ -3892,6 +3894,8 @@ RValue CallEmission::applyNormalCall(SGFContext C) {
   calleeTypeInfo.substResultType = formalType.getResult();
 
   if (selfArg.hasValue() && callSite.hasValue()) {
+    calleeTypeInfo.origFormalType =
+        calleeTypeInfo.origFormalType->getFunctionResultType();
     calleeTypeInfo.origResultType =
       calleeTypeInfo.origResultType->getFunctionResultType();
     calleeTypeInfo.substResultType =
@@ -4382,7 +4386,9 @@ RValue SILGenFunction::emitApply(
     // we left during the first pass.
     auto &completionArgSlot = const_cast<ManagedValue &>(args[completionIndex]);
 
-    completionArgSlot = resultPlan->emitForeignAsyncCompletionHandler(*this, loc);
+    auto origFormalType = *calleeTypeInfo.origFormalType;
+    completionArgSlot = resultPlan->emitForeignAsyncCompletionHandler(
+        *this, origFormalType, loc);
 
   } else if (auto foreignError = calleeTypeInfo.foreign.error) {
     unsigned errorParamIndex =
diff --git a/lib/SILGen/SILGenBridging.cpp b/lib/SILGen/SILGenBridging.cpp
@@ -2215,6 +2215,8 @@ void SILGenFunction::emitForeignToNativeThunk(SILDeclRef thunk) {
         foreignError,
         foreignAsync,
         ImportAsMemberStatus());
+    calleeTypeInfo.origFormalType =
+        foreignCI.FormalPattern.getFunctionResultType();
 
     auto init = indirectResult
                 ? useBufferAsTemporary(indirectResult,
diff --git a/lib/SILGen/SILGenThunk.cpp b/lib/SILGen/SILGenThunk.cpp
@@ -182,15 +182,28 @@ static const clang::Type *prependParameterType(
   return clangCtx.getPointerType(newFnTy).getTypePtr();
 }
 
-SILFunction *
-SILGenModule::getOrCreateForeignAsyncCompletionHandlerImplFunction(
-                                         CanSILFunctionType blockType,
-                                         CanType continuationTy,
-                                         CanGenericSignature sig,
-                                         ForeignAsyncConvention convention) {
+SILFunction *SILGenModule::getOrCreateForeignAsyncCompletionHandlerImplFunction(
+    CanSILFunctionType blockType, CanType continuationTy,
+    AbstractionPattern origFormalType, CanGenericSignature sig,
+    ForeignAsyncConvention convention) {
   // Extract the result and error types from the continuation type.
   auto resumeType = cast<BoundGenericType>(continuationTy).getGenericArgs()[0];
 
+  CanAnyFunctionType completionHandlerOrigTy = [&]() {
+    auto completionHandlerOrigTy =
+        origFormalType.getObjCMethodAsyncCompletionHandlerForeignType(convention, Types);
+    Optional<CanAnyFunctionType> maybeCompletionHandlerOrigTy;
+    if (auto fnTy =
+            dyn_cast<AnyFunctionType>(completionHandlerOrigTy)) {
+      maybeCompletionHandlerOrigTy = fnTy;
+    } else {
+      maybeCompletionHandlerOrigTy = cast<AnyFunctionType>(
+          completionHandlerOrigTy.getOptionalObjectType());
+    }
+    return maybeCompletionHandlerOrigTy.getValue();
+  }();
+  auto blockParams = completionHandlerOrigTy.getParams();
+
   // Build up the implementation function type, which matches the
   // block signature with an added block storage argument that points at the
   // block buffer. The block storage holds the continuation we feed the
@@ -208,7 +221,7 @@ SILGenModule::getOrCreateForeignAsyncCompletionHandlerImplFunction(
       getASTContext(),
       blockType->getClangTypeInfo().getType(),
       getASTContext().getClangTypeForIRGen(blockStorageTy));
-  
+
   auto implTy = SILFunctionType::get(sig,
          blockType->getExtInfo().intoBuilder()
            .withRepresentation(SILFunctionTypeRepresentation::CFunctionPointer)
@@ -356,15 +369,13 @@ SILGenModule::getOrCreateForeignAsyncCompletionHandlerImplFunction(
         auto resumeArgBuf = SGF.emitTemporaryAllocation(loc,
                                               loweredResumeTy.getAddressType());
 
-        auto prepareArgument = [&](SILValue destBuf, ManagedValue arg) {
+        auto prepareArgument = [&](SILValue destBuf, CanType destFormalType,
+                                   ManagedValue arg, CanType argFormalType) {
           // Convert the ObjC argument to the bridged Swift representation we
           // want.
-          ManagedValue bridgedArg = SGF.emitBridgedToNativeValue(loc,
-                                       arg.copy(SGF, loc),
-                                       arg.getType().getASTType(),
-                                       // FIXME: pass down formal type
-                                       destBuf->getType().getASTType(),
-                                       destBuf->getType().getObjectType());
+          ManagedValue bridgedArg = SGF.emitBridgedToNativeValue(
+              loc, arg.copy(SGF, loc), argFormalType, destFormalType,
+              destBuf->getType().getObjectType());
           // Force-unwrap an argument that comes to us as Optional if it's
           // formally non-optional in the return.
           if (bridgedArg.getType().getOptionalObjectType()
@@ -388,19 +399,40 @@ SILGenModule::getOrCreateForeignAsyncCompletionHandlerImplFunction(
             continue;
           paramIndices.push_back(index);
         }
+        auto blockParamIndex = [paramIndices](unsigned long i) {
+          // The non-error, non-flag block parameter (formal types of the
+          // completion handler's arguments) indices are the same as the the
+          // parameter (lowered types of the completion handler's arguments)
+          // indices but shifted by 1 corresponding to the fact that the lowered
+          // completion handler has a block_storage argument but the formal type
+          // does not.
+          return paramIndices[i] - 1;
+        };
         if (auto resumeTuple = dyn_cast<TupleType>(resumeType)) {
           assert(paramIndices.size() == resumeTuple->getNumElements());
           assert(params.size() == resumeTuple->getNumElements()
                                    + 1 + (bool)errorIndex + (bool)flagIndex);
           for (unsigned i : indices(resumeTuple.getElementTypes())) {
             auto resumeEltBuf = SGF.B.createTupleElementAddr(loc,
                                                              resumeArgBuf, i);
-            prepareArgument(resumeEltBuf, params[paramIndices[i]]);
+            prepareArgument(
+                /*destBuf*/ resumeEltBuf,
+                /*destFormalType*/
+                F->mapTypeIntoContext(resumeTuple.getElementTypes()[i])
+                    ->getCanonicalType(),
+                /*arg*/ params[paramIndices[i]],
+                /*argFormalType*/
+                blockParams[blockParamIndex(i)].getParameterType());
           }
         } else {
           assert(paramIndices.size() == 1);
           assert(params.size() == 2 + (bool)errorIndex + (bool)flagIndex);
-          prepareArgument(resumeArgBuf, params[paramIndices[0]]);
+          prepareArgument(/*destBuf*/ resumeArgBuf,
+                          /*destFormalType*/
+                          F->mapTypeIntoContext(resumeType)->getCanonicalType(),
+                          /*arg*/ params[paramIndices[0]],
+                          /*argFormalType*/
+                          blockParams[blockParamIndex(0)].getParameterType());
         }
         
         // Resume the continuation with the composed bridged result.
diff --git a/test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h b/test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h
@@ -57,6 +57,13 @@ typedef void (^CompletionHandler)(NSString * _Nullable, NSString * _Nullable_res
 // rdar://73798726
 - (void)getSomeObjectWithCompletionHandler:(nullable void (^)(NSObject *_Nullable x, NSError *_Nullable error))handler;
 
+- (void)performVoid2VoidWithCompletion:(void (^ _Nonnull)(void (^ _Nonnull)(void)))completion;
+- (void)performId2VoidWithCompletion:(void (^ _Nonnull)(void (^ _Nonnull)(id _Nonnull)))completion;
+- (void)performId2IdWithCompletion:(void (^ _Nonnull)(id _Nonnull (^ _Nonnull)(id _Nonnull)))completion;
+- (void)performNSString2NSStringWithCompletion:(void (^ _Nonnull)(NSString * _Nonnull (^ _Nonnull)(NSString * _Nonnull)))completion;
+- (void)performNSString2NSStringNSStringWithCompletion:(void (^ _Nonnull)(NSString * _Nonnull (^ _Nonnull)(NSString * _Nonnull), NSString * _Nonnull))completion;
+- (void)performId2VoidId2VoidWithCompletion:(void (^ _Nonnull)(void (^ _Nonnull)(id _Nonnull), void (^ _Nonnull)(id _Nonnull)))completion;
+
 -(void)oldAPIWithCompletionHandler:(void (^ _Nonnull)(NSString *_Nullable, NSError *_Nullable))handler __attribute__((availability(macosx, deprecated=10.14)));
 
 -(void)someAsyncMethodWithBlock:(void (^ _Nonnull)(NSString *_Nullable, NSError *_Nullable))completionHandler;
diff --git a/test/SILGen/objc_async.swift b/test/SILGen/objc_async.swift
@@ -76,6 +76,13 @@ func testSlowServer(slowServer: SlowServer) async throws {
   let _: NSObject? = try await slowServer.stopRecording()
   let _: NSObject = try await slowServer.someObject()
 
+  let _: () -> Void = await slowServer.performVoid2Void()
+  let _: (Any) -> Void = await slowServer.performId2Void()
+  let _: (Any) -> Any = await slowServer.performId2Id()
+  let _: (String) -> String = await slowServer.performNSString2NSString()
+
+  let _: ((String) -> String, String) = await slowServer.performNSString2NSStringNSString()
+  let _: ((Any) -> Void, (Any) -> Void) = await slowServer.performId2VoidId2Void()
 }
 
 func testGeneric<T: AnyObject>(x: GenericObject<T>) async throws {
diff --git a/validation-test/compiler_crashers_2_fixed/rdar79383990.swift b/validation-test/compiler_crashers_2_fixed/rdar79383990.swift
@@ -0,0 +1,9 @@
+// RUN: %target-swift-frontend %s -emit-silgen -disable-availability-checking
+// REQUIRES: objc_interop
+// REQUIRES: OS=macosx
+
+import Foundation
+
+func test(s: NSBackgroundActivityScheduler) async {
+    _ = await s.schedule()
+}

Original file line number	Diff line number	Diff line change
`@@ -2784,7 +2784,7 @@ SILType GetAsyncContinuationInstBase::getLoweredResumeType() const {`
`2784`	`2784`	`auto formalType = getFormalResumeType();`
`2785`	`2785`	`auto &M = getFunction()->getModule();`
`2786`	`2786`	`auto c = getFunction()->getTypeExpansionContext();`
`2787`		`- return M.Types.getLoweredType(AbstractionPattern::getOpaque(), formalType, c);`
	`2787`	`+ return M.Types.getLoweredType(AbstractionPattern(formalType), formalType, c);`
`2788`	`2788`	`}`
`2789`	`2789`
`2790`	`2790`	`ReturnInst::ReturnInst(SILFunction &func, SILDebugLocation debugLoc,`
Original file line number	Diff line number	Diff line change
`@@ -49,9 +49,9 @@ class ResultPlan {`
`49`	`49`	`emitForeignErrorArgument(SILGenFunction &SGF, SILLocation loc) {`
`50`	`50`	`return None;`
`51`	`51`	`}`
`52`		`-`
`53`		`- virtual ManagedValue`
`54`		`- emitForeignAsyncCompletionHandler(SILGenFunction &SGF, SILLocation loc) {`
	`52`	`+`
	`53`	`+ virtual ManagedValue emitForeignAsyncCompletionHandler(`
	`54`	`+ SILGenFunction &SGF, AbstractionPattern origFormalType, SILLocation loc) {`
`55`	`55`	`return {};`
`56`	`56`	`}`
`57`	`57`	`};`