Skip to content

Commit 3040136

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 910d918 commit 3040136

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
@@ -261,19 +261,23 @@ SILGenModule::getOrCreateForeignAsyncCompletionHandlerImplFunction(
261261
auto continuationVal = SGF.B.createLoad(loc, continuationAddr,
262262
LoadOwnershipQualifier::Trivial);
263263
auto continuation = ManagedValue::forUnmanaged(continuationVal);
264-
264+
265265
// Check for an error if the convention includes one.
266-
auto errorIndex = convention.completionHandlerErrorParamIndex();
267-
auto flagIndex = convention.completionHandlerFlagParamIndex();
266+
// Increment the error and flag indices if present. They do not account
267+
// for the fact that they are preceded by the block_storage arguments.
268+
auto errorIndex = convention.completionHandlerErrorParamIndex().map(
269+
[](auto original) { return original + 1; });
270+
auto flagIndex = convention.completionHandlerFlagParamIndex().map(
271+
[](auto original) { return original + 1; });
268272

269273
FuncDecl *resumeIntrinsic;
270274

271275
SILBasicBlock *returnBB = nullptr;
272276
if (errorIndex) {
273277
resumeIntrinsic = getResumeUnsafeThrowingContinuation();
274278
auto errorIntrinsic = getResumeUnsafeThrowingContinuationWithError();
275-
276-
auto errorArgument = params[*errorIndex + 1];
279+
280+
auto errorArgument = params[*errorIndex];
277281
auto someErrorBB = SGF.createBasicBlock(FunctionSection::Postmatter);
278282
auto noneErrorBB = SGF.createBasicBlock();
279283
returnBB = SGF.createBasicBlockAfter(noneErrorBB);
@@ -282,8 +286,8 @@ SILGenModule::getOrCreateForeignAsyncCompletionHandlerImplFunction(
282286
// Check whether there's an error, based on the presence of a flag
283287
// parameter. If there is a flag parameter, test it against zero.
284288
if (flagIndex) {
285-
auto flagArgument = params[*flagIndex + 1];
286-
289+
auto flagArgument = params[*flagIndex];
290+
287291
// The flag must be an integer type. Get the underlying builtin
288292
// integer field from it.
289293
auto builtinFlagArg = SGF.emitUnwrapIntegerResult(loc, flagArgument.getValue());
@@ -378,31 +382,31 @@ SILGenModule::getOrCreateForeignAsyncCompletionHandlerImplFunction(
378382
bridgedArg.forwardInto(SGF, loc, destBuf);
379383
};
380384

385+
// Collect the indices which correspond to the values to be returned.
386+
SmallVector<unsigned long, 4> paramIndices;
387+
for (auto index : indices(params)) {
388+
// The first index is the block_storage parameter.
389+
if (index == 0)
390+
continue;
391+
if (errorIndex && index == *errorIndex)
392+
continue;
393+
if (flagIndex && index == *flagIndex)
394+
continue;
395+
paramIndices.push_back(index);
396+
}
381397
if (auto resumeTuple = dyn_cast<TupleType>(resumeType)) {
398+
assert(paramIndices.size() == resumeTuple->getNumElements());
382399
assert(params.size() == resumeTuple->getNumElements()
383400
+ 1 + (bool)errorIndex + (bool)flagIndex);
384401
for (unsigned i : indices(resumeTuple.getElementTypes())) {
385-
unsigned paramI = i;
386-
if (errorIndex && paramI >= *errorIndex) {
387-
++paramI;
388-
}
389-
if (flagIndex && paramI >= *flagIndex) {
390-
++paramI;
391-
}
392402
auto resumeEltBuf = SGF.B.createTupleElementAddr(loc,
393403
resumeArgBuf, i);
394-
prepareArgument(resumeEltBuf, params[paramI + 1]);
404+
prepareArgument(resumeEltBuf, params[paramIndices[i]]);
395405
}
396406
} else {
407+
assert(paramIndices.size() == 1);
397408
assert(params.size() == 2 + (bool)errorIndex + (bool)flagIndex);
398-
unsigned paramI = 0;
399-
if (errorIndex && paramI >= *errorIndex) {
400-
++paramI;
401-
}
402-
if (flagIndex && paramI >= *flagIndex) {
403-
++paramI;
404-
}
405-
prepareArgument(resumeArgBuf, params[paramI + 1]);
409+
prepareArgument(resumeArgBuf, params[paramIndices[0]]);
406410
}
407411

408412
// 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)