Skip to content

Commit 95142cf

Browse files
committed
[SILGen] Precompute "returned" completion handler indices.
When generating the completion handler that is passed to ObjC methods that are being called as async Swift functions, the parameters taken by the generated completion handler are matched up with the values that are returned from the async function. Previously, this process was done by starting at the index into the list of values expected to be returned, adding 1 if the index matched first the error index and second the flag index, and finally adding 1 to account for the fact that the initial parameter to the completion handler is the block_storage parameter. That strategy was problematic: it failed to increment indices appropriately because it did not skip past the block_storage parameter to begin with; it also failed to increment indices appropriately in certain cases where the flag and error parameters appeared in an unexpected order (flag first, then error). Here, the process is made more robust. Now, the indices which correspond to the block_storage, flag, and error parameters are filtered out to begin with. The remaining indices can then be indexed into using the index into the result tuple (or 0 if the result is not a tuple). rdar://80847020
1 parent 0d3e6f3 commit 95142cf

File tree

4 files changed

+67
-23
lines changed

4 files changed

+67
-23
lines changed

lib/SILGen/SILGenThunk.cpp

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -255,19 +255,23 @@ SILGenModule::getOrCreateForeignAsyncCompletionHandlerImplFunction(
255255
auto continuationVal = SGF.B.createLoad(loc, continuationAddr,
256256
LoadOwnershipQualifier::Trivial);
257257
auto continuation = ManagedValue::forUnmanaged(continuationVal);
258-
258+
259259
// Check for an error if the convention includes one.
260-
auto errorIndex = convention.completionHandlerErrorParamIndex();
261-
auto flagIndex = convention.completionHandlerFlagParamIndex();
260+
// Increment the error and flag indices if present. They do not account
261+
// for the fact that they are preceded by the block_storage arguments.
262+
auto errorIndex = convention.completionHandlerErrorParamIndex().map(
263+
[](auto original) { return original + 1; });
264+
auto flagIndex = convention.completionHandlerFlagParamIndex().map(
265+
[](auto original) { return original + 1; });
262266

263267
FuncDecl *resumeIntrinsic;
264268

265269
SILBasicBlock *returnBB = nullptr;
266270
if (errorIndex) {
267271
resumeIntrinsic = getResumeUnsafeThrowingContinuation();
268272
auto errorIntrinsic = getResumeUnsafeThrowingContinuationWithError();
269-
270-
auto errorArgument = params[*errorIndex + 1];
273+
274+
auto errorArgument = params[*errorIndex];
271275
auto someErrorBB = SGF.createBasicBlock(FunctionSection::Postmatter);
272276
auto noneErrorBB = SGF.createBasicBlock();
273277
returnBB = SGF.createBasicBlockAfter(noneErrorBB);
@@ -276,8 +280,8 @@ SILGenModule::getOrCreateForeignAsyncCompletionHandlerImplFunction(
276280
// Check whether there's an error, based on the presence of a flag
277281
// parameter. If there is a flag parameter, test it against zero.
278282
if (flagIndex) {
279-
auto flagArgument = params[*flagIndex + 1];
280-
283+
auto flagArgument = params[*flagIndex];
284+
281285
// The flag must be an integer type. Get the underlying builtin
282286
// integer field from it.
283287
auto builtinFlagArg = SGF.emitUnwrapIntegerResult(loc, flagArgument.getValue());
@@ -372,31 +376,31 @@ SILGenModule::getOrCreateForeignAsyncCompletionHandlerImplFunction(
372376
bridgedArg.forwardInto(SGF, loc, destBuf);
373377
};
374378

379+
// Collect the indices which correspond to the values to be returned.
380+
SmallVector<unsigned long, 4> paramIndices;
381+
for (auto index : indices(params)) {
382+
// The first index is the block_storage parameter.
383+
if (index == 0)
384+
continue;
385+
if (errorIndex && index == *errorIndex)
386+
continue;
387+
if (flagIndex && index == *flagIndex)
388+
continue;
389+
paramIndices.push_back(index);
390+
}
375391
if (auto resumeTuple = dyn_cast<TupleType>(resumeType)) {
392+
assert(paramIndices.size() == resumeTuple->getNumElements());
376393
assert(params.size() == resumeTuple->getNumElements()
377394
+ 1 + (bool)errorIndex + (bool)flagIndex);
378395
for (unsigned i : indices(resumeTuple.getElementTypes())) {
379-
unsigned paramI = i;
380-
if (errorIndex && paramI >= *errorIndex) {
381-
++paramI;
382-
}
383-
if (flagIndex && paramI >= *flagIndex) {
384-
++paramI;
385-
}
386396
auto resumeEltBuf = SGF.B.createTupleElementAddr(loc,
387397
resumeArgBuf, i);
388-
prepareArgument(resumeEltBuf, params[paramI + 1]);
398+
prepareArgument(resumeEltBuf, params[paramIndices[i]]);
389399
}
390400
} else {
401+
assert(paramIndices.size() == 1);
391402
assert(params.size() == 2 + (bool)errorIndex + (bool)flagIndex);
392-
unsigned paramI = 0;
393-
if (errorIndex && paramI >= *errorIndex) {
394-
++paramI;
395-
}
396-
if (flagIndex && paramI >= *flagIndex) {
397-
++paramI;
398-
}
399-
prepareArgument(resumeArgBuf, params[paramI + 1]);
403+
prepareArgument(resumeArgBuf, params[paramIndices[0]]);
400404
}
401405

402406
// Resume the continuation with the composed bridged result.
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#import <Foundation/Foundation.h>
2+
3+
#pragma clang assume_nonnull begin
4+
5+
@interface Clazz : NSObject
6+
-(void)doSomethingMultiResultFlaggyWithCompletionHandler:(void (^)(BOOL, NSString *_Nullable, NSError *_Nullable, NSString *_Nullable))completionHandler __attribute__((swift_async_error(zero_argument, 1)));
7+
@end
8+
9+
#pragma clang assume_nonnull end
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#include "rdar80847020.h"
2+
3+
#pragma clang assume_nonnull begin
4+
5+
@implementation Clazz
6+
-(void)doSomethingMultiResultFlaggyWithCompletionHandler:(void (^)(BOOL, NSString *_Nullable, NSError *_Nullable, NSString *_Nullable))completionHandler __attribute__((swift_async_error(zero_argument, 1))) {
7+
completionHandler(YES, @"hi", NULL, @"bye");
8+
}
9+
@end
10+
11+
#pragma clang assume_nonnull end

test/Interpreter/rdar80847020.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-clang %S/Inputs/rdar80847020.m -I %S/Inputs -c -o %t/rdar80847020.o
3+
// RUN: %target-build-swift -import-objc-header %S/Inputs/rdar80847020.h -Xlinker %t/rdar80847020.o -parse-as-library %s -o %t/a.out
4+
// RUN: %target-codesign %t/a.out
5+
// RUN: %target-run %t/a.out | %FileCheck %s
6+
7+
// REQUIRES: executable_test
8+
// REQUIRES: objc_interop
9+
10+
func run(_ s: Clazz) async throws {
11+
let res: (String, String) = try await s.doSomethingMultiResultFlaggy()
12+
// CHECK: ("hi", "bye")
13+
print(res)
14+
}
15+
16+
@main struct Main {
17+
static func main() async throws {
18+
try await run(Clazz())
19+
}
20+
}

0 commit comments

Comments
 (0)