-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Use Disjunction Constraint to find main function #58429
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
Use Disjunction Constraint to find main function #58429
Conversation
@swift-ci please test |
This reverts commit da0a331.
The main function is different from other function resolutions. It isn't being called from a synchronous or asynchronous context, but defines whether the program starts in a synchronous or async context. As a result, we should not prefer one over the other, so the scoring mechanism shouldn't involve the async/sync score when resolving the main function. This patch adds a constraint solver flag to ignore async/sync context mismatches so that we do not favor one over the other, but otherwise use the normal resolution behavior.
Using `resolveValueMember` can result in the wrong, or unrelated main functions being selected. If an error occurs while resolving, such as an ambiguous resolution, the solution will contain everything called 'main'. During the resolution, the `BestOverload` will not be set, so the code skips down to iterating over the member decls and selecting the first declaration that is a `MainTypeMainMethod`. The order of candidates in the failure state is related to the order that the declarations appear in source. The selected and potentially unrelated main function is called from `$main`. The call in the expression will then detect the invalid state later. When only a single main function could exist in a given context, this was not a problem. Any other possible solutions would result in an error, either due to a duplicate declaration, or from calling an unrelated function. When we introduced the asynchronous main function, the resolution can be ambiguous because we allow asynchronous overloads of synchronous functions. This enables us to legally write ``` struct MainType { static func main() { } static func main() async { } } ``` From the perspective of duplicate declarations, this is not an issue. It is, however, ambiguous since we have no calling context with which to break the tie. In the original code, this example would select the synchronous main function because it comes first in the source. If we flip the declarations, the asynchronous main function is selected because it comes first in the source. Instead, using the constraint solver to solve for the correct overload will ensure that we only get back a valid main function. If the constraint solver reports no solutions or many solutions, we can emit an error immediately, as it will not consider unrelated functions.
Changing the overload resolution behavior results in the ordering that `main` and `$main` are typechecked. If there is an error in parsing or sema before we have typechecked these two functions, we get weird errors about `$main` calling the main-actor `main` function being invalid. This is because we have already set the `@main` attribute to invalid, so we don't recognize the `$main` context as being a main entrypoint. The error message compounds to making a confusing situation worse given that `$main` is implicit. So by ignoring the failed `@main` attribute, we still annotate the `$main` and `main` function as being on the main actor, so everyone is happy in that respect. To get the nasty behavior, you can forget to pass `-parse-as-library` with this commit removed.
This patch contains the updates for the tests. The merge removes the `-async-main` flag, so the tests using that flag fail on that. Additionally, the tests reflect the newer behavior of the main resolution. `async_main_resolution` verifies that we're not preferring an async function over a synchronous function, and vice versa. This is also verified in `where_clause_main_resolution`, where we select a main function using various configuration typealiases. Finally, we only select a valid, usable main function. The old machinery could select one sort of at random (technically it selected the first overload declared in the source file) if an error occurred. Errors that resulted in this behavior included a missing valid function, or an ambiguous overload. In the case of the missing valid function, the function returned would be from an invalid location if one existed, which was called from `$main`. `$main` had sufficient context to realize that the main function being called was not valid, and would emit an error saying why. It would be better to realize that we're not getting a valid main function earlier, and later we can emit notes saying why each function called main could not be selected.
5407c96
to
1ed8556
Compare
@swift-ci please test |
@swift-ci please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look
// CHECK-CONFIG3: [[SOURCE_FILE]]:[[# @LINE+1 ]] | ||
static func main() { } | ||
extension App { | ||
static func main() async { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the only expected behavior change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's more behavior change here than just the ambiguous resolution with both main functions.
The bigger change here is that we always resolve the most specific main function regardless of whether it's async or sync without needing the flag to bias things.
This sets up the type constraints as equalities with a type variable. This is apparently more performant in the solver, so that should be good. It doesn't have additional effect on the resulting behavior.
28459a6
to
b9405bb
Compare
@swift-ci please test |
Third time trying to get the main function resolution behaving correctly.
The challenge here is that, unlike other functions, we don't have a way to determine whether the "calling context" of the main function is synchronous or asynchronous because there isn't a calling context. It is up to the main function to determine whether the rest of the program is run from a sync or async context.
The last two designs didn't work correctly for various reasons.
Simply making the constraint solver ignore async/sync mismatches results in incorrect behavior with conditional conformances. The following snippet exposed the issue. The original resolver would fall back on grabbing the first candidate from the list of candidates if no best solution was found. A best solution wouldn't be set if there were ambiguities or missing functions. The candidates were in the order of declaration in the source file. So in this case, if you selected configuration 3, it would see the ambiguity between the synchronous and asynchronous main function, and select the main function in config 1. (This is bad!) It should instead report the ambiguity.
To mitigate this, I tried using an
async-main
flag, which would bias the constraint solver in favor of one over the other.This was not ideal for a few reasons. While it resolved the above case with the explicit flag, it first, required an explicit flag that would completely change the behavior of the program, but second, would prefer either the sync or async function of the specific type. e.g, in the above case, if you specified
Config1
, with the async flag selected, it would choose the default async main function instead of the function fromConfig1
.In order to actually fix this to be properly sane, I've done a bit of digging with the constraint solver. Rather than dragging out everything called
main
, then trawling the list of everything we found, I've set up a disjunction constraint of only the types and names of valid main functions. This way we only get a list of valid function declarations, with a valid type, calledmain
. The valid types are() -> Void
,() async -> Void
,() throws -> Void
,() async throws -> Void
,@MainActor () -> Void
,@MainActor () async -> Void
,@MainActor () throws -> Void
, and@MainActor () async throws -> Void
.