Skip to content

Make namelookup not allergic to async main #40577

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

Conversation

etcwilde
Copy link
Member

@etcwilde etcwilde commented Dec 16, 2021

The typesystem was avoiding the async main function if any synchronous
main was available. This is because the decl contexts where the main
function is looked up from aren't async contexts (extension contexts,
generic type contexts), so if a synchronous main function exists
anywhere, it will be a better "fit" for the context.

Since we're the main function, we decide on whether or not we want to be
async, so looking at the decl-context's asyncness is really the wrong
thing to do. Plumbing this info through the constraint system seems
risky at best, catastrophic at worst, so instead, I'm just grabbing the
buffer of resolved decls and doing my own scoring and selection.

We want to ensure that we are picking the main function that is closest
to the decl marked with @main in the protocol conformance chain. This
means that if your @main struct directly conforms to a protocol that
implements a main function, you should probably use it. If that protocol
implements two main functions, one sync and one async, we look at
whether we support async before selecting it.

Things get a little bit weird if you have implemented an async main in
the closest protocol, but are targeting something that doesn't support
concurrency. The @available checking should handle this with a nice
error saying you're doing bad things, but if you circumvent that
checking for bad reasons, it will still fall back on a synchronous main
function if one is available.

https://bugs.swift.org/browse/SR-15211
rdar://83316060

@etcwilde etcwilde added the concurrency Feature: umbrella label for concurrency language features label Dec 16, 2021
@etcwilde
Copy link
Member Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 5f57e9dd7991feefee1c9dd4b52fe34a25bcbb9c

@etcwilde
Copy link
Member Author

Expecting this to fail on macOS too.
There are a bunch of tests with deployment targets that are below where concurrency was introduced. They used to "happen to work", but now are failing because I'm only resolving async main function decls when concurrency is available here.

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 5f57e9dd7991feefee1c9dd4b52fe34a25bcbb9c

@etcwilde etcwilde force-pushed the ewilde/concurrency/update-main-namelookup-resolution branch from 5f57e9d to cdbc1df Compare December 17, 2021 17:26
@etcwilde
Copy link
Member Author

@swift-ci please test macOS

The typesystem was avoiding the async main function if any synchronous
main was available. This is because the decl contexts where the main
function is looked up from aren't async contexts (extension contexts,
generic type contexts), so if a synchronous main function exists
anywhere, it will be a better "fit" for the context.

Since we're the main function, we decide on whether or not we want to be
async, so looking at the decl-context's asyncness is really the wrong
thing to do. Plumbing this info through the constraint system seems
risky at best, catastrophic at worst, so instead, I'm just grabbing the
buffer of resolved decls and doing my own scoring and selection.

We want to ensure that we are picking the main function that is closest
to the decl marked with `@main` in the protocol conformance chain. This
means that if your `@main` struct directly conforms to a protocol that
implements a main function, you should probably use it. If that protocol
implements two main functions, one sync and one async, we look at
whether we support async before selecting it.

Things get a little bit weird if you have implemented an async main in
the closest protocol, but are targeting something that doesn't support
concurrency. The `@available` checking should handle this with a nice
error saying you're doing bad things, but if you circumvent that
checking for bad reasons, it will still fall back on a synchronous main
function if one is available.
@etcwilde etcwilde force-pushed the ewilde/concurrency/update-main-namelookup-resolution branch from cdbc1df to 46d4517 Compare December 17, 2021 18:35
@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - cdbc1df040018a4299938a282913755b081d64c7

We won't look for async functions that can't be used due to availability
unless the availability checking is disabled. Need to disable the
availability checking due to the minimum deploy target being too low for
concurrency.
@etcwilde
Copy link
Member Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - a53c727

Fixing testing for different platforms. MacOS and iOS have
backdeployment and availability to consider. Other platforms don't need
to worry about that because they need to include the swift runtime in
the installation package anyway, so the concurrency bits can ship with
the installation.
@etcwilde
Copy link
Member Author

@swift-ci please test

@etcwilde etcwilde marked this pull request as ready for review December 21, 2021 16:31
Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

This looks great!

@etcwilde etcwilde merged commit df6b279 into swiftlang:main Dec 21, 2021
etcwilde added a commit to etcwilde/swift that referenced this pull request Feb 26, 2022
This is effectively reverting swiftlang#40577.
The mechanism that I tried to use didn't respect `where` clauses on
extensions and, as a result, could select a totally unrelated and
incorrect main function. This results in beautiful explosions in the
rest of the typesystem.
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.

3 participants