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

Fix records type inference rules to match implementations. #3613

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

stereotype441
Copy link
Member

If the context of a record literal is the same shape as the record literal, but the actual type of one or more fields is not a subtype of the corresponding field in the context type, and no coercion is possible, then the implementations do not necessarily issue an error. They only issue an error if the type mismatch leads to an assignability error during subsequent analysis.

An example of a circumstance where the type mismatch doesn't lead to an assignability error is if the context resulted from assignment to a previously promoted local variable. For example:

f(Object o) {
  if (o is (int,)) {
    o = ('',); // OK; demotes `o` back to `Object`.
  }
}

This change adjusts the spec to agree with what was implemented.


  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

If the context of a record literal is the same shape as the record
literal, but the actual type of one or more fields is not a subtype of
the corresponding field in the context type, and no coercion is
possible, then the implementations do not necessarily issue an
error. They only issue an error if the type mismatch leads to an
assignability error during subsequent analysis.

An example of a circumstance where the type mismatch doesn't lead to
an assignability error is if the context resulted from assignment to a
previously promoted local variable. For example:

    f(Object o) {
      if (o is (int,)) {
        o = ('',); // OK; demotes `o` back to `Object`.
      }
    }

This change adjusts the spec to agree with what was implemented.
@munificent
Copy link
Member

This change looks good to me, but @leafpetersen wrote the inference section for records and he'll understand much better than I would if there are complications here that I'm not seeing.

copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Feb 13, 2024
When a local variable is promoted to a record type, and then an
assignment statement is used to assign a record literal to that local
variable, if the fields of the new record literal are not assignable
to the fields of the promoted record type, that's not a problem; both
the analyzer and front end agree that the local variable is simply
demoted.

But the spec implies that if the old and new record _shapes_ are the
same, then a compile-time error will occur instead of a demotion. I've
created dart-lang/language#3613 to bring the
spec in line with the implementations. This test demonstrates the
current behavior of the implementations, and makes sure that it
doesn't regress.

Change-Id: I0eacd7ca7f6579a35dbc34687113a2112418f368
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/352462
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Copy link
Member

@leafpetersen leafpetersen left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@stereotype441 stereotype441 merged commit 31c665c into main Feb 14, 2024
2 of 3 checks passed
@stereotype441 stereotype441 deleted the schema_16 branch February 14, 2024 00:39
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.

None yet

3 participants