Skip to content

Commit d3f644a

Browse files
committed
[SILGen] fix crash invoking global-actor qualified optional ObjC async method
Due to some changes in how the AST is constructed by CSApply to handle global actors from @preconcurrency declarations, the AST for a force-call to an optional ObjC method that is part of a global-actor qualified protocol, such as: ``` import Foundation @mainactor @objc protocol P { @objc optional func generateMaybe() async } extension P { func test() async -> Void { await self.generateMaybe!() } } ``` now contains a function conversion in between the forced-value operation and the dynamic-ref: ``` (force_value_expr type='@mainactor () async -> ()' (optional_evaluation_expr implicit type='(@mainactor () async -> ())?' (inject_into_optional implicit type='(@mainactor () async -> ())?' (function_conversion_expr implicit type='@mainactor () async -> ()' <<<<< new! (bind_optional_expr implicit type='() async -> ()' (dynamic_member_ref_expr type='(() async -> ())?' ... ))))) ``` For compatibility with how ObjC does message sends to these optional methods, in Swift there is a difference between how something like `a.method!()` and the following are compiled: ``` let meth = a.method! meth() ``` The former will directly call the optional method and let the "unknown selector" error be emitted if it's not implemented. But the latter will query the dynamic method and inject that result into an `Optional<...>` to then be forced, yielding a "force-unwrap nil" error. It's a subtle difference but those two scenarios lead to different code paths in SILGen. Now, this patch fixes the issue by enhancing the recognition of these direct call scenarios so that it can look past these function conversions. Because that recognition already tries to ignore implicit conversions, this is not something new. The problem was that we weren't considering implicit conversions between the optional-evaluation and bind-optional expressions. This patch is intentionally precise about what kind of function conversions can be skipped, because I do not know if all of them are OK to blindly skip or not. We might need to expand that in the future. Resolves rdar://97646309
1 parent bd999a5 commit d3f644a

File tree

1 file changed

+43
-1
lines changed

1 file changed

+43
-1
lines changed

lib/SILGen/SILGenApply.cpp

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1595,6 +1595,39 @@ class SILGenApply : public Lowering::ExprVisitor<SILGenApply> {
15951595
return std::move(*applyCallee);
15961596
}
15971597

1598+
/// \returns true if the conversion is from an async function to
1599+
/// the same type but with a global actor added. For example this:
1600+
/// () async -> () ==> @MainActor () async -> ()
1601+
/// will return true. In all other cases, returns false.
1602+
static bool addsGlobalActorToAsyncFn(FunctionConversionExpr *fce) {
1603+
CanType oldTy = fce->getSubExpr()->getType()->getCanonicalType();
1604+
CanType newTy = fce->getType()->getCanonicalType();
1605+
1606+
if (auto oldFnTy = dyn_cast<AnyFunctionType>(oldTy)) {
1607+
if (auto newFnTy = dyn_cast<AnyFunctionType>(newTy)) {
1608+
// old type MUST be async
1609+
if (!oldFnTy->hasEffect(EffectKind::Async))
1610+
return false;
1611+
1612+
// old type MUST NOT have a global actor
1613+
if (oldFnTy->hasGlobalActor())
1614+
return false;
1615+
1616+
// new type MUST have a global actor
1617+
if (!newFnTy->hasGlobalActor())
1618+
return false;
1619+
1620+
// see if adding the global actor to the old type yields the new type.
1621+
auto globalActor = newFnTy->getGlobalActor();
1622+
auto addedActor = oldFnTy->getExtInfo().withGlobalActor(globalActor);
1623+
1624+
return oldFnTy->withExtInfo(addedActor) == newFnTy;
1625+
}
1626+
}
1627+
1628+
return false;
1629+
}
1630+
15981631
/// Ignore parentheses and implicit conversions.
15991632
static Expr *ignoreParensAndImpConversions(Expr *expr) {
16001633
while (true) {
@@ -1608,7 +1641,16 @@ class SILGenApply : public Lowering::ExprVisitor<SILGenApply> {
16081641
// works given that we check the result for certain forms.
16091642
if (auto eval = dyn_cast<OptionalEvaluationExpr>(expr)) {
16101643
if (auto inject = dyn_cast<InjectIntoOptionalExpr>(eval->getSubExpr())) {
1611-
if (auto bind = dyn_cast<BindOptionalExpr>(inject->getSubExpr())) {
1644+
1645+
auto nextSubExpr = inject->getSubExpr();
1646+
1647+
// skip over a specific, known no-op function conversion, if it exists
1648+
if (auto funcConv = dyn_cast<FunctionConversionExpr>(nextSubExpr)) {
1649+
if (addsGlobalActorToAsyncFn(funcConv))
1650+
nextSubExpr = funcConv->getSubExpr();
1651+
}
1652+
1653+
if (auto bind = dyn_cast<BindOptionalExpr>(nextSubExpr)) {
16121654
if (bind->getDepth() == 0)
16131655
return bind->getSubExpr();
16141656
}

0 commit comments

Comments
 (0)