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: align front end behavior with analyzer for if-null expressions in context dynamic. #3650

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

Comments

@stereotype441
Copy link
Member

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 a subtle difference between the analyzer and front end type inference of if-null expressions. This issue addresses that 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 if-null expression.

When inferring an if-null expression e1 ?? e2 in a context of dynamic, the front end analyzes both e1 and e2 with a context of dynamic. Whereas the analyzer analyzes e1 with a context of _, and e2 with a context of T1, where T1 is the static type of e1.

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, however such a program would have been rejected by the analyzer, so this should only crop up in workflows where the analyzer is not used (e.g. a user who doesn't use an IDE, and doesn't run the analyzer as part of a continuous integration setup). It's also conceivable that this change could cause a change in the behavior of a compiled program by changing a reified type parameter.

To reduce the risk of causing a suprising customer breakage, I'm happy to validate this change against google3, and to push it through the breaking change process.

@dart-lang/language-team any objections?

@lrhn
Copy link
Member

lrhn commented Mar 14, 2024

No objection.

Although I do want to question why dynamic is considered "no context" but Object? isn't.

My guess is that it's because dynamic can occur as a result of instantiate to bounds, so the user might not have written it, in which case we don't want to hold them to it.
Or maybe it just felt different, as a "no type requirement" (which is how we treat dynamic in general, it's the absence of a static type).

Could we choose to go the other way, and make both the analyzer do what the front end does instead? What would the cost be? Someone would get dynamic as the type of an expression in a context where they already had dynamic as a type?

(Could we go further and respect dynamic as an explicit type everywhere, and only use _ for actually no context type?)

@eernstg
Copy link
Member

eernstg commented Mar 15, 2024

No objections here, either. Seems to be the consistent resolution of the discrepancy.

@lrhn lrhn added the feature Proposed language feature that solves one or more problems label Mar 18, 2024
@leafpetersen
Copy link
Member

SGTM

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue May 8, 2024
…lyzer.

Fixes dart-lang/language#3650.
Fixes #55436.

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

Completed via dart-lang/sdk@5b020f2.

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

4 participants