reimplement MapEntry as a record

[RohitSaily]

As stated by its documentation, MapEntry has been planned to be changed to a value type. This change achieves that by making it a Record (K, V). Backwards compatibility is mostly achieved by preserving the original API through an extension type and ensuring it implements Object. See #61071 (comment) for what would not be backwards compatible

asPair is added to the API to allow direct usage as a record, it was required anyways since it is the backing field

[copybara-service]

Thank you for your contribution! This project uses Gerrit for code reviews. Your pull request has automatically been converted into a code review at:

https://dart-review.googlesource.com/c/sdk/+/439100

Please wait for a developer to review your code review at the above link; you can speed up the review if you sign into Gerrit and manually add a reviewer that has recently worked on the relevant code. See CONTRIBUTING.md to learn how to upload changes to Gerrit directly.

Additional commits pushed to this PR will update both the PR and the corresponding Gerrit CL. After the review is complete on the CL, your reviewer will merge the CL (automatically closing this PR).

[RohitSaily]

Now the record constructor is accessible and named fromPair

[copybara-service]

https://dart-review.googlesource.com/c/sdk/+/439100 has been updated with the latest commits from this pull request.

[copybara-service]

https://dart-review.googlesource.com/c/sdk/+/439100 has been updated with the latest commits from this pull request.

[RohitSaily]

cc @bwilkerson @keertip

[mraleph]

See also discussions on #54965

This is technically a breaking change so it requires breaking change process, and even before that it requires @lrhn to pre-approve the design.

[copybara-service]

https://dart-review.googlesource.com/c/sdk/+/439100 has been updated with the latest commits from this pull request.

[copybara-service]

https://dart-review.googlesource.com/c/sdk/+/439100 has been updated with the latest commits from this pull request.

[RohitSaily]

@mraleph thanks for linking that and tagging @lrhn. This change uses an extension type and includes the original API as a subset of its API* and therefore should not be breaking

* the _ constructor is removed but that is internal and was not used anywhere, so I simply omitted it since it was not needed

[mraleph]

It is breaking because it changes runtime semantics: the difference between old and new version is observable via is / as checks as well as through equality and identity of MapEntry (original class did not override ==).

That being said the breakage is probably very limited - MapEntry should only really occur as temporary objects.

[RohitSaily]

It is breaking because it changes runtime semantics: the difference between old and new version is observable via is / as checks as well as through equality and identity of MapEntry (original class did not override ==).

That being said the breakage is probably very limited - MapEntry should only really occur as temporary objects.

Good points, I did not consider those things. The original class-based MapEntry is replaced in this change so any MapEntry is/as behaviour should still be preserved for MapEntry but if (K, V) or Record happens to be checked or casted it would now work differently, as you have said eg

(1, 'hello') is MapEntry<int, String> // used to be false, now true
(1, 'hello') as MapEntry<int, String> // used to throw Error, now it works

f (MapEntry<int, String> e) {
  e is Record  // used to be false, now true
  e is (int, String)  // used to be false, now true
}

Also agree this changes == to the record defined behaviour from the original identity based one

[copybara-service]

https://dart-review.googlesource.com/c/sdk/+/439100 has been updated with the latest commits from this pull request.

[copybara-service]

https://dart-review.googlesource.com/c/sdk/+/439100 has been updated with the latest commits from this pull request.

[copybara-service]

https://dart-review.googlesource.com/c/sdk/+/439100 has been updated with the latest commits from this pull request.

[copybara-service]

https://dart-review.googlesource.com/c/sdk/+/439100 has been updated with the latest commits from this pull request.

[copybara-service]

https://dart-review.googlesource.com/c/sdk/+/439100 has been updated with the latest commits from this pull request.

[lrhn]

It's not the time, yet.

The Dart 3.0 language changes to the platform libraries, including adding final to MapEntry, were made conditional on the language version of the client library.

That means that pre-3.0 language version libraries can extend MapEntry, and changing the class to an extension type will break that.
So it's a breaking change as long as the SDK supports language version 2.19.

Other than that, I don't mind MapEntry having equality. In fact, I thought I did that already.

I wouldn't expose asPair.
I would consider using a ({K key, V value}) as representation record, so that dynamic access would keep working.

But not yet.