Skip to content
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

Adjust function literal return type inference #3149

Closed
wants to merge 5 commits into from

Conversation

eernstg
Copy link
Member

@eernstg eernstg commented Jun 15, 2023

Issue #3148 showed that we need to adjust the rules about function literal return type inference. This PR changes the rules accordingly.

The basic idea is that the last step in the inference of an async function used to use the inferred return type of Future<flatten(S)> where S is taken from the context or from the returned expressions, but it should instead be Future<S>. Also, flatten then needs to be applied to the type obtained from the context, such that we still have the same return type as before when the context type gets to decide the outcome.

If the previous paragraph did not yield a value for `S`: When the function is
marked `async`: if `T <: futureValueType(R)` then let `S` be `T`; otherwise let
`S` be `futureValueType(R)`. When the function is not marked `async`: if `T <:
R` then let `S` be `T`; otherwise let `S` be `R`.
Copy link
Member Author

Choose a reason for hiding this comment

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

We need to use futureValueType(R), not flatten(R). If we were to use flatten then we'd carry ? over from the context. So we could have a context type that wants a Future<num>?, then we'd get int? as the value of R, and then we'd get an inferred return type of Future<int?> where the context wants a return type of Future<int>?, and that's a type error.

Future<int>? Function() f = () async => 1;

In this example R is Future<int>? and flatten(R) is int?. But we don't want the function literal to have a return type of Future<int?>, it should be Future<int>.

@eernstg
Copy link
Member Author

eernstg commented Jun 15, 2023

Closing: #3151 contains a PR which simplifies a larger number of locations in this section because it avoids wrapping K in FutureOr in the first place, and that's an overall improvement.

@eernstg eernstg closed this Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant