Skip to content

Commit cbe87a4

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 11e3d00 commit cbe87a4

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
@@ -1592,6 +1592,39 @@ class SILGenApply : public Lowering::ExprVisitor<SILGenApply> {
15921592
return std::move(*applyCallee);
15931593
}
15941594

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

0 commit comments

Comments
 (0)