Skip to content

Fix SILOptimizer crash for top-level async code #41187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 4, 2022

Conversation

etcwilde
Copy link
Member

@etcwilde etcwilde commented Feb 3, 2022

Lowering hops to actors expected the executor to either be an optional
builtin executor type or an actor type. The type coming from
getMainExecutor is just a builtin executor, not an optional. As a
result, LowerHopToActor::emitGetExecutor would get called in
LowerHopToActor::processHop, and would try looking up the builtin
executor type, thinking it was an actor type. This would fail because we
didn't have an actor type.

auto actorConf = module->lookupConformance(actorType, actorProtocol);
assert(actorConf &&
           "hop_to_executor with actor that doesn't conform to Actor");

The end result was hitting this assert here, saying that the "actor"
doesn't conform to actor.

The fix is to wrap the executor in an optional.

Lowering hops to actors expected the executor to either be an optional
builtin executor type or an actor type. The type coming from
`getMainExecutor` is just a builtin executor, not an optional. As a
result, `LowerHopToActor::emitGetExecutor` would get called in
`LowerHopToActor::processHop`, and would try looking up the builtin
executor type, thinking it was an actor type. This would fail because we
didn't have an actor type.

```
auto actorConf = module->lookupConformance(actorType, actorProtocol);
    assert(actorConf &&
           "hop_to_executor with actor that doesn't conform to Actor");
```

The end result was hitting this assert here, saying that the "actor"
doesn't conform to actor.

The fix is to wrap the executor in an optional.
@etcwilde etcwilde added the concurrency Feature: umbrella label for concurrency language features label Feb 3, 2022
@etcwilde etcwilde requested a review from rjmccall February 3, 2022 19:18
@etcwilde
Copy link
Member Author

etcwilde commented Feb 3, 2022

@swift-ci please test

Copy link
Contributor

@rjmccall rjmccall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@etcwilde etcwilde merged commit b26a575 into swiftlang:main Feb 4, 2022
@etcwilde etcwilde deleted the ewilde/fix-toplevel-async branch February 4, 2022 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concurrency Feature: umbrella label for concurrency language features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants