[SILGen] Handled foreign funcs with async, error, and more.

nate-chandler · nate-chandler · commit bc09cc9cf8d8 · 2021-10-28T13:42:08.000-07:00
In 1ae317d, all the machinery was added to enable calling ObjC functions that have both async and error conventions. The handling that was added for matching up arguments and parameters, however, failed to handle functions that had both foreign async and foreign error and other arguments. Here, that handling is fixed. The fix is in four parts: - Reverting to a single call to CallSite::emit to emit the formal params. At this point, the foreign async and/or error params will be claimed as well. - Separately counting the async and error parameters in ParamLowering::claimParams to ensure we get the right parameter slices. - Letting ArgEmitter::maybeEmitForeignArgument look for an error parameter even if it already found an async parameter. - Letting ArgEmitter::maybeEmitForeignArgument keep looking for foreign arguments after it finds one. rdar://80704984
diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp
@@ -3299,26 +3299,33 @@ class ArgEmitter {
   }
   
   void maybeEmitForeignArgument() {
-    if (Foreign.async
-        && Foreign.async->completionHandlerParamIndex() == Args.size()) {
-      SILParameterInfo param = claimNextParameters(1).front();
-      (void)param;
-
-      // Leave a placeholder in the position. We'll fill this in with a block
-      // capturing the current continuation right before we invoke the
-      // function.
-      // (We can't do this immediately, because evaluating other arguments
-      // may require suspending the async task, which is not allowed while its
-      // continuation is active.)
-      Args.push_back(ManagedValue::forInContext());
-    } else if (Foreign.error
-               && Foreign.error->getErrorParameterIndex() == Args.size()) {
-      SILParameterInfo param = claimNextParameters(1).front();
-      assert(param.getConvention() == ParameterConvention::Direct_Unowned);
-      (void) param;
-
-      // Leave a placeholder in the position.
-      Args.push_back(ManagedValue::forInContext());
+    bool keepGoing = true;
+    while (keepGoing) {
+      keepGoing = false;
+      if (Foreign.async &&
+          Foreign.async->completionHandlerParamIndex() == Args.size()) {
+        SILParameterInfo param = claimNextParameters(1).front();
+        (void)param;
+
+        // Leave a placeholder in the position. We'll fill this in with a block
+        // capturing the current continuation right before we invoke the
+        // function.
+        // (We can't do this immediately, because evaluating other arguments
+        // may require suspending the async task, which is not allowed while its
+        // continuation is active.)
+        Args.push_back(ManagedValue::forInContext());
+        keepGoing = true;
+      }
+      if (Foreign.error &&
+          Foreign.error->getErrorParameterIndex() == Args.size()) {
+        SILParameterInfo param = claimNextParameters(1).front();
+        assert(param.getConvention() == ParameterConvention::Direct_Unowned);
+        (void)param;
+
+        // Leave a placeholder in the position.
+        Args.push_back(ManagedValue::forInContext());
+        keepGoing = true;
+      }
     }
   }
 
@@ -3541,7 +3548,9 @@ struct ParamLowering {
       }
     }
 
-    if (foreign.error || foreign.async)
+    if (foreign.error)
+      ++count;
+    if (foreign.async)
       ++count;
 
     if (foreign.self.isImportAsMember()) {
@@ -4202,18 +4211,12 @@ ApplyOptions CallEmission::emitArgumentsForNormalApply(
 
     args.push_back({});
 
-    if (!foreign.async || (foreign.async && foreign.error)) {
-      // Claim the foreign error argument(s) with the method formal params.
-      auto siteForeignError = ForeignInfo{{}, foreign.error, {}};
-      std::move(*callSite).emit(SGF, origFormalType, substFnType, paramLowering,
-                                args.back(), delayedArgs, siteForeignError);
-    }
-    if (foreign.async) {
-      // Claim the foreign async argument(s) with the method formal params.
-      auto siteForeignAsync = ForeignInfo{{}, {}, foreign.async};
-      std::move(*callSite).emit(SGF, origFormalType, substFnType, paramLowering,
-                                args.back(), delayedArgs, siteForeignAsync);
-    }
+    // Claim the foreign error and/or async arguments when claiming the formal
+    // params.
+    auto siteForeignError = ForeignInfo{{}, foreign.error, foreign.async};
+    // Claim the method formal params.
+    std::move(*callSite).emit(SGF, origFormalType, substFnType, paramLowering,
+                              args.back(), delayedArgs, siteForeignError);
   }
 
   uncurriedLoc = callSite->Loc;
diff --git a/validation-test/compiler_crashers_2_fixed/Inputs/rdar80704984_3.h b/validation-test/compiler_crashers_2_fixed/Inputs/rdar80704984_3.h
@@ -0,0 +1,28 @@
+#include <Foundation/Foundation.h>
+
+#pragma clang assume_nonnull begin
+
+typedef void (^CompletionHandler)(int status, NSInteger bytesTransferred);
+
+@interface PFXObject : NSObject
+- (BOOL)enqueueFailingRequestWithData:(nullable NSMutableData *)data
+                                error:(NSError *_Nullable *)error
+                    completionHandler:
+                        (nullable CompletionHandler)completionHandler;
+- (BOOL)enqueuePassingRequestWithData:(nullable NSMutableData *)data
+                                error:(NSError *_Nullable *)error
+                    completionHandler:
+                        (nullable CompletionHandler)completionHandler;
+- (BOOL)enqueueFailingRequestWithData:(nullable NSMutableData *)data
+                    completionTimeout:(NSTimeInterval)completionTimeout
+                                error:(NSError *_Nullable *)error
+                    completionHandler:
+                        (nullable CompletionHandler)completionHandler;
+- (BOOL)enqueuePassingRequestWithData:(nullable NSMutableData *)data
+                    completionTimeout:(NSTimeInterval)completionTimeout
+                                error:(NSError *_Nullable *)error
+                    completionHandler:
+                        (nullable CompletionHandler)completionHandler;
+@end
+
+#pragma clang assume_nonnull end
diff --git a/validation-test/compiler_crashers_2_fixed/Inputs/rdar80704984_3.m b/validation-test/compiler_crashers_2_fixed/Inputs/rdar80704984_3.m
@@ -0,0 +1,45 @@
+#include "rdar80704984_3.h"
+
+#pragma clang assume_nonnull begin
+
+@implementation PFXObject
+
+- (BOOL)enqueueFailingRequestWithData:(nullable NSMutableData *)data
+                                error:(NSError *_Nullable *)error
+                    completionHandler:
+                        (nullable CompletionHandler)completionHandler {
+  *error = [[NSError alloc] initWithDomain:@"d" code:1 userInfo:nil];
+  return NO;
+}
+- (BOOL)enqueuePassingRequestWithData:(nullable NSMutableData *)data
+                                error:(NSError *_Nullable *)error
+                    completionHandler:
+                        (nullable CompletionHandler)completionHandler {
+  dispatch_async(dispatch_get_main_queue(), ^{
+    completionHandler(0, 2);
+  });
+  return YES;
+}
+
+- (BOOL)enqueueFailingRequestWithData:(nullable NSMutableData *)data
+                    completionTimeout:(NSTimeInterval)completionTimeout
+                                error:(NSError *_Nullable *)error
+                    completionHandler:
+                        (nullable CompletionHandler)completionHandler {
+  *error = [[NSError alloc] initWithDomain:@"d" code:2 userInfo:nil];
+  return NO;
+}
+- (BOOL)enqueuePassingRequestWithData:(nullable NSMutableData *)data
+                    completionTimeout:(NSTimeInterval)completionTimeout
+                                error:(NSError *_Nullable *)error
+                    completionHandler:
+                        (nullable CompletionHandler)completionHandler {
+  dispatch_async(dispatch_get_main_queue(), ^{
+    completionHandler(0, 3);
+  });
+  return YES;
+}
+
+@end
+
+#pragma clang assume_nonnull end
diff --git a/validation-test/compiler_crashers_2_fixed/rdar80704984_3.swift b/validation-test/compiler_crashers_2_fixed/rdar80704984_3.swift
@@ -0,0 +1,54 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-clang %S/Inputs/rdar80704984_3.m -I %S/Inputs -c -o %t/rdar80704984_3.o
+// RUN: %target-build-swift -Xfrontend -disable-availability-checking -import-objc-header %S/Inputs/rdar80704984_3.h -Xlinker %t/rdar80704984_3.o -parse-as-library %s -o %t/a.out
+// RUN: %target-codesign %t/a.out
+// RUN: %target-run %t/a.out | %FileCheck %s
+
+// REQUIRES: executable_test
+// REQUIRES: objc_interop
+
+// rdar://82123254
+// UNSUPPORTED: use_os_stdlib
+// UNSUPPORTED: back_deployment_runtime
+
+func run1(on object: PFXObject) async throws {
+  do {
+    _ = try await object.enqueueFailingRequest(with: nil)
+  }
+  catch let error {
+    // CHECK: Domain=d Code=1 
+    print(error)
+  }
+}
+
+func run2(on object: PFXObject) async throws {
+    // CHECK: (0, 2)
+    print(try await object.enqueuePassingRequest(with: nil))
+  
+}
+
+func run3(on object: PFXObject) async throws {
+  do {
+    _ = try await object.enqueueFailingRequest(with: nil, completionTimeout: 57.0)
+  }
+  catch let error {
+    // CHECK: Domain=d Code=2 
+    print(error)
+  }
+}
+
+func run4(on object: PFXObject) async throws {
+    // CHECK: (0, 3)
+    print(try await object.enqueuePassingRequest(with: nil, completionTimeout: 57.0))
+  
+}
+
+@main struct Main {
+  static func main() async throws {
+    let object = PFXObject()
+    try await run1(on: object)
+    try await run2(on: object)
+    try await run3(on: object)
+    try await run4(on: object)
+  }
+}
