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

Proposal: remove special front end behavior for await expressions with context dynamic. #3649

Closed
stereotype441 opened this issue Mar 11, 2024 · 3 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@stereotype441
Copy link
Member

stereotype441 commented Mar 11, 2024

Background: I'm trying to update the expression inference rules in inference.md to match what was implemented, so that we can have a rigorous specification of the inference-update-3 logic I landed in https://dart-review.googlesource.com/c/sdk/+/353440. In the process I've discovered some subtle differences between the analyzer and front end type inference of await expressions. This issue addresses one such difference.

In the analyzer, type inferring an expression in a context dynamic is always considered equivalent to type inferring it in a context _. This is because the analyzer uses the method TypeAnalyzer.analyzeExpression whenever it supplies a context when type inferring an expression, and TypeAnalyzer.analyzeExpression changes a context of dynamic to a context of _.

The front end also has logic that changes a context of dynamic to a context of _, but this logic isn't used for all expression types; it is only used for generic invocations and expressions appearing inside pattern constructs (such as the expressions inside a switch expression). I believe there are only two circumstances in which the difference is observable: when inferring an await expression and when inferring an if-null expression. This issue is about the observable difference when inferring an await expression.

When inferring an await expression in a context of dynamic, the front end analyzes the await expression's subexpression using a context of dynamic. Whereas the analyzer analyzes the same await expression's subexpression using a context of FutureOr<_>.

For the most part, the way contexts feed into the type inference algorithm is by appearing to the right of <# in the subtype constraint generation algorithm. This algorithm treats a right hand side of dynamic nearly identically to a right hand side of FutureOr<_>, so it is difficult to come up with an example of code that the analyzer and front end treat differently.

But we can exploit the special behavior of e1 ?? e2, which behaves as follows: if it is type inferred in context _, then e2 is type inferred using the static type of e1 as its context; whereas if it is type inferred in context K, then e2 is type inferred using K as its context.

Here is an example program that is analyzed differently:

Future<T> g<T>(T t) => Future.value(t);
T h<T>(T t) => t;

extension on Future<int> {
  void foo() {}
}

test(Future<num>? f) async {
  dynamic x = await h(f ?? (g(0)..foo()));
}

main() {}

This example is accepted by the analyzer, because:

  • await h(f ?? g(0)..foo())) is inferred using a context of dynamic.
  • Therefore, h(f ?? g(0)..foo())) is inferred using a context of FutureOr<_>.
  • Therefore, f ?? (g(0)..foo()) is inferred using a context of FutureOr<_>.
  • Therefore, g(0)..foo() is inferred using a context of FutureOr<_>.
  • Therefore, g(0) is inferred using a context of FutureOr<_>.
  • Since the return type of g is Future<T>, and that satisfies FutureOr<_> for all T, downwards inference of g(0) does not constrain the type of T. So the type of T is set to int during upwards inference.
  • Therefore, g(0) has static type Future<int>.
  • Therefore, ..foo() resolves to the extension method foo defined in extension on Future<int>.

But it is rejected by the front end, because:

  • await h(f ?? (g(0)..foo())) is inferred using a context of dynamic.
  • Therefore, h(f ?? g(0)..foo())) is inferred using a context of dynamic.
  • Since h is a generic invocation, the context dynamic is changed to _, so f ?? (g(0)..foo()) is inferred using a context of _.
  • Therefore, g(0)..foo() is inferred using a context of Future<num>? (the static type of f).
  • Therefore, g(0) is inferred using a context of Future<num>?.
  • Since the return type of g is Future<T>, and that satisfies FutureOr<num>? only if T <: num, the type of T is set to num during downwards inference.
  • Therefore, g(0) has static type Future<num>.
  • Therefore, ..foo() does not resolve to the extension method foo defined in extension on Future<int>.

Whereas this program is accepted by both the analyzer and the front end:

T h<T>(T t) => t;

extension on Future<int> {
  void foo() {}
}

test(Future<num>? f) async {
  var x = await h(f ?? (g(0)..foo()));
}

main() {}

(The only difference is that the variable x has been changed to be implicitly typed rather than having an explicit type of dynamic, so the await expression is now analyzed using a context of _).

I propose to base my update of the expression inference rules in inference.md on the analyzer's behavior in this corner case, and to change the front end's behavior to match the analyzer's.

This change could in principle cause the front end to reject a program that it previously accepted, or to cause a change in the behavior of a compiled program by changing a reified type parameter. But I believe it will be extremely unlikely to produce an observable difference in practice, because contexts of _ and FutureOr<_> are treated so similarly by type inference.

Nonetheless, I'm happy to push this change through the breaking change process.

@dart-lang/language-team any objections?

@stereotype441
Copy link
Member Author

Note: I've separately proposed, in #3571, changing the inference rules for await expressions to reduce the proliferation of unnecessary FutureOrs in contexts. That change is much more potentially breaking than this one (and should probably be done in a language-versioned way), so I'm hoping to move forward with this change first.

@lrhn
Copy link
Member

lrhn commented Mar 14, 2024

I have no problem with this, but as for ??, I'll question why dynamic is considered "no type context" to begin with.

@lrhn lrhn added the feature Proposed language feature that solves one or more problems label Mar 18, 2024
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue May 8, 2024
Fixes dart-lang/language#3649.
Fixes #55418.

Bug: dart-lang/language#3649
Change-Id: Ifb2fe47bb343a357e2338843775f140c01bd8a88
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/361302
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
@stereotype441
Copy link
Member Author

Completed via dart-lang/sdk@497d610.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems
Projects
None yet
Development

No branches or pull requests

2 participants