Skip to content

Commit ce651ca

Browse files
committed
[SILGen] ResultPlan: Address a FIXME about resume with foreign error
1 parent 6ecbb08 commit ce651ca

File tree

2 files changed

+78
-38
lines changed

2 files changed

+78
-38
lines changed

lib/SILGen/ResultPlan.cpp

Lines changed: 48 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -719,7 +719,11 @@ class ForeignAsyncInitializationPlan final : public ResultPlan {
719719
SILValue resumeBuf;
720720
SILValue continuation;
721721
ExecutorBreadcrumb breadcrumb;
722-
722+
723+
SILValue blockStorage;
724+
CanType blockStorageTy;
725+
CanType continuationTy;
726+
723727
public:
724728
ForeignAsyncInitializationPlan(SILGenFunction &SGF, SILLocation loc,
725729
const CalleeTypeInfo &calleeTypeInfo)
@@ -820,9 +824,6 @@ class ForeignAsyncInitializationPlan final : public ResultPlan {
820824
continuation = SGF.B.createGetAsyncContinuationAddr(loc, resumeBuf,
821825
calleeTypeInfo.substResultType, throws);
822826

823-
SILValue blockStorage;
824-
CanType blockStorageTy;
825-
CanType continuationTy;
826827
std::tie(blockStorage, blockStorageTy, continuationTy) =
827828
emitBlockStorage(SGF, loc, throws);
828829

@@ -907,65 +908,74 @@ class ForeignAsyncInitializationPlan final : public ResultPlan {
907908
// (1) fulfill the unsafe continuation with the foreign error
908909
// (2) branch to the await block
909910
{
910-
// First, fulfill the unsafe continuation with the foreign error.
911+
// First, fulfill the continuation with the foreign error.
911912
// Currently, that block's code looks something like
912913
// %foreignError = ... : $*Optional<NSError>
913914
// %converter = function_ref _convertNSErrorToError(_:)
914915
// %error = apply %converter(%foreignError)
915916
// [... insert here ...]
916917
// destroy_value %error
917918
// destroy_value %foreignError
918-
// Insert code to fulfill it after the native %error is defined. That
919-
// code should structure the RawUnsafeContinuation (continuation) into
920-
// an appropriately typed UnsafeContinuation and then pass that together
921-
// with (a copy of) the error to
922-
// _resumeUnsafeThrowingContinuationWithError.
919+
// Insert code to fulfill it after the native %error is defined. That
920+
// code should load UnsafeContinuation (or CheckedContinuation
921+
// depending on mode) and then pass that together with (a copy of) the
922+
// error to _resume{Unsafe, Checked}ThrowingContinuationWithError.
923923
// [foreign_error_block_with_foreign_async_convention]
924924
SGF.B.setInsertionPoint(
925925
++bridgedForeignError->getDefiningInstruction()->getIterator());
926926

927-
// FIXME: this case is not respecting checked bridging, and it's a
928-
// great candidate for that. This situation comes up when bridging
929-
// to an ObjC completion-handler method that returns a bool. It seems
930-
// that bool indicates whether the handler was invoked. If it was not
931-
// then it writes out an error. Here for the unsafe bridging, we're
932-
// invoking the continuation by re-wrapping it in an
933-
// UnsafeContinuation<_, Error> and then immediately calling its
934-
// resume(throwing: error) method. For a checked bridging scenario, we
935-
// would need to use a copy of the original CheckedContinuation that
936-
// was passed to the callee. Whether that's by invoking the block
937-
// ourselves, or just invoking the CheckedContinuation.
938-
939-
auto continuationDecl = ctx.getUnsafeContinuationDecl();
940-
auto errorTy = ctx.getErrorExistentialType();
941-
auto continuationBGT =
942-
BoundGenericType::get(continuationDecl, Type(),
943-
{calleeTypeInfo.substResultType, errorTy});
927+
bool checkedBridging = ctx.LangOpts.UseCheckedAsyncObjCBridging;
928+
944929
auto env = SGF.F.getGenericEnvironment();
945930
auto sig = env ? env->getGenericSignature().getCanonicalSignature()
946931
: CanGenericSignature();
947-
auto mappedContinuationTy =
948-
continuationBGT->mapTypeOutOfContext()->getReducedType(sig);
932+
933+
// Load unsafe or checked continuation from the block storage
934+
// and call _resume{Unsafe, Checked}ThrowingContinuationWithError.
935+
936+
SILValue continuationAddr =
937+
SGF.B.createProjectBlockStorage(loc, blockStorage);
938+
939+
ManagedValue continuation;
940+
if (checkedBridging) {
941+
FormalEvaluationScope scope(SGF);
942+
943+
auto underlyingValueTy =
944+
OpenedArchetypeType::get(ctx.TheAnyType, sig);
945+
946+
auto underlyingValueAddr = SGF.emitOpenExistential(
947+
loc, ManagedValue::forTrivialAddressRValue(continuationAddr),
948+
SGF.getLoweredType(underlyingValueTy), AccessKind::Read);
949+
950+
continuation = SGF.B.createUncheckedAddrCast(
951+
loc, underlyingValueAddr,
952+
SILType::getPrimitiveAddressType(continuationTy));
953+
} else {
954+
auto continuationVal = SGF.B.createLoad(
955+
loc, continuationAddr, LoadOwnershipQualifier::Trivial);
956+
continuation =
957+
ManagedValue::forObjectRValueWithoutOwnership(continuationVal);
958+
}
959+
960+
auto mappedOutContinuationTy =
961+
continuationTy->mapTypeOutOfContext()->getReducedType(sig);
949962
auto resumeType =
950-
cast<BoundGenericType>(mappedContinuationTy).getGenericArgs()[0];
951-
auto continuationTy = continuationBGT->getCanonicalType();
963+
cast<BoundGenericType>(mappedOutContinuationTy).getGenericArgs()[0];
952964

953965
auto errorIntrinsic =
954-
SGF.SGM.getResumeUnsafeThrowingContinuationWithError();
966+
checkedBridging
967+
? SGF.SGM.getResumeCheckedThrowingContinuationWithError()
968+
: SGF.SGM.getResumeUnsafeThrowingContinuationWithError();
969+
955970
Type replacementTypes[] = {
956971
SGF.F.mapTypeIntoContext(resumeType)->getCanonicalType()};
957972
auto subs = SubstitutionMap::get(errorIntrinsic->getGenericSignature(),
958973
replacementTypes,
959974
ArrayRef<ProtocolConformanceRef>{});
960-
auto wrappedContinuation = SGF.B.createStruct(
961-
loc, SILType::getPrimitiveObjectType(continuationTy),
962-
{continuation});
963975

964-
auto continuationMV = ManagedValue::forObjectRValueWithoutOwnership(
965-
SILValue(wrappedContinuation));
966976
SGF.emitApplyOfLibraryIntrinsic(
967977
loc, errorIntrinsic, subs,
968-
{continuationMV,
978+
{continuation,
969979
SGF.B.copyOwnedObjectRValue(loc, bridgedForeignError,
970980
ManagedValue::ScopeKind::Lexical)},
971981
SGFContext());

test/SILGen/objc_async_checked.swift

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,36 @@ func testSlowServerFromMain(slowServer: SlowServer) async throws {
217217
let _: Int = await slowServer.doSomethingSlow("mail")
218218
}
219219

220+
// CHECK-LABEL: sil hidden [ossa] @$s18objc_async_checked20testWithForeignError10slowServerySo04SlowI0C_tYaKF : $@convention(thin) @async (@guaranteed SlowServer) -> @error any Error
221+
// CHECK: [[STR:%.*]] = alloc_stack $String
222+
// CHECK: [[ERR:%.*]] = alloc_stack [dynamic_lifetime] $Optional<NSError>
223+
// CHECK: inject_enum_addr [[ERR]] : $*Optional<NSError>, #Optional.none!enumelt
224+
// CHECK: [[METHOD:%.*]] = objc_method %0 : $SlowServer, #SlowServer.findAnswerFailingly!foreign : (SlowServer) -> () async throws -> String, $@convention(objc_method) (Optional<AutoreleasingUnsafeMutablePointer<Optional<NSError>>>, @convention(block) (Optional<NSString>, Optional<NSError>) -> (), SlowServer) -> ObjCBool
225+
// CHECK: [[RAW_CONT:%.*]] = get_async_continuation_addr [throws] String, {{.*}} : $*String
226+
// CHECK: [[UNSAFE_CONT:%.*]] = struct $UnsafeContinuation<String, any Error> ([[RAW_CONT]] : $Builtin.RawUnsafeContinuation)
227+
// CHECK: [[BLOCK_STORAGE:%.*]] = alloc_stack $@block_storage Any
228+
// CHECK: [[PROJECTED:%.*]] = project_block_storage [[BLOCK_STORAGE]] : $*@block_storage Any
229+
// CHECK: [[CHECKED_CONT_SLOT:%.*]] = init_existential_addr [[PROJECTED]] : $*Any, $CheckedContinuation<String, any Error>
230+
// CHECK: [[CHECKED_CONT_INIT_FN:%.*]] = function_ref @$ss34_createCheckedThrowingContinuationyScCyxs5Error_pGSccyxsAB_pGnlF : $@convention(thin) <τ_0_0> (UnsafeContinuation<τ_0_0, any Error>) -> @out CheckedContinuation<τ_0_0, any Error>
231+
// CHECK: [[CHECKED_CONT:%.*]] = alloc_stack $CheckedContinuation<String, any Error>
232+
// CHECK: {{.*}} = apply [[CHECKED_CONT_INIT_FN]]<String>([[CHECKED_CONT]], [[UNSAFE_CONT]]) : $@convention(thin) <τ_0_0> (UnsafeContinuation<τ_0_0, any Error>) -> @out CheckedContinuation<τ_0_0, any Error>
233+
// CHECK: copy_addr [take] [[CHECKED_CONT]] to [init] [[CHECKED_CONT_SLOT]] : $*CheckedContinuation<String, any Error>
234+
// CHECK: cond_br {{.*}}, [[RESUME:bb[0-9]+]], [[ERROR:bb[0-9]+]]
235+
//
236+
// CHECK: [[ERROR]]:
237+
// CHECK: [[ERR_VALUE:%.*]] = load [take] [[ERR]] : $*Optional<NSError>
238+
// CHECK: [[CONV_FN:%.*]] = function_ref @$s10Foundation22_convertNSErrorToErrorys0E0_pSo0C0CSgF : $@convention(thin) (@guaranteed Optional<NSError>) -> @owned any Error
239+
// CHECK: [[SWIFT_ERROR:%.*]] = apply [[CONV_FN]]([[ERR_VALUE]]) : $@convention(thin) (@guaranteed Optional<NSError>) -> @owned any Error
240+
// CHECK: [[PROJECTED_STORAGE:%.*]] = project_block_storage [[BLOCK_STORAGE]] : $*@block_storage Any
241+
// CHECK: [[CONT_OPENED:%.*]] = open_existential_addr immutable_access [[PROJECTED_STORAGE]] : $*Any to $*@opened("{{.*}}", Any) Self
242+
// CHECK: [[CHECKED_CONT:%.*]] = unchecked_addr_cast [[CONT_OPENED]] : $*@opened("{{.*}}", Any) Self to $*CheckedContinuation<String, any Error>
243+
// CHECK: [[ERROR_COPY:%.*]] = copy_value [[SWIFT_ERROR]] : $any Error
244+
// CHECK: [[RESUME_FN:%.*]] = function_ref @$ss43_resumeCheckedThrowingContinuationWithErroryyScCyxs0F0_pG_sAB_pntlF : $@convention(thin) <τ_0_0> (@in_guaranteed CheckedContinuation<τ_0_0, any Error>, @owned any Error) -> ()
245+
// CHECK: {{.*}} = apply [[RESUME_FN]]<String>([[CHECKED_CONT]], [[ERROR_COPY]]) : $@convention(thin) <τ_0_0> (@in_guaranteed CheckedContinuation<τ_0_0, any Error>, @owned any Error) -> ()
246+
func testWithForeignError(slowServer: SlowServer) async throws {
247+
let _: String = try await slowServer.findAnswerFailingly()
248+
}
249+
220250
// CHECK-LABEL: sil {{.*}}@${{.*}}26testThrowingMethodFromMain
221251
@MainActor
222252
func testThrowingMethodFromMain(slowServer: SlowServer) async -> String {

0 commit comments

Comments
 (0)