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

[breaking change] Change the superinterface of IntX #59776

Open
eernstg opened this issue Dec 20, 2024 · 3 comments
Open

[breaking change] Change the superinterface of IntX #59776

eernstg opened this issue Dec 20, 2024 · 3 comments
Assignees
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. breaking-change-request This tracks requests for feedback on breaking changes

Comments

@eernstg
Copy link
Member

eernstg commented Dec 20, 2024

Change Intent

IntX implements Comparable<IntX> rather than Comparable<Object>.

Justification

Currently, Int64 and Int32 have no supertype K such that K extends Comparable<K>. This causes all invocations of methods like sortedBy to fail. Inference fails, and it is also impossible to specify the actual type argument explicitly because there is no solution to the given constraint K extends Comparable<K>.

The change proposed here makes IntX a solution to the constraint, which means that it can be specified manually. Moreover, it can be inferred automatically starting with Dart 3.7.0 (because it has the new feature proposed in dart-lang/language#3009).

Impact

No impact is expected.

Of course, it is always possible to find an expression that breaks (assert(Int64(1) is! Comparable<IntX>)), but the change did not give rise to any failures in a TAP Presubmit in internal code, and there are no known examples of breakage with any practical relevance.

Mitigation

No mitigation needed. Developers may wish to use sortedBy and similar methods in situations where it was previously not possible, but this is an option, not a necessity.

Change Timeline

ASAP.

Associated CLs

https://dart-review.googlesource.com/c/core/+/401680

@eernstg eernstg added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. breaking-change-request This tracks requests for feedback on breaking changes labels Dec 20, 2024
@eernstg
Copy link
Member Author

eernstg commented Dec 20, 2024

FYI @lrhn, @itsjustkevin.

@kevmoo
Copy link
Member

kevmoo commented Dec 22, 2024

SGTM!

@lrhn
Copy link
Member

lrhn commented Dec 22, 2024

Have to decide whether it's a major version increment (in which var is allowed to be breaking, but might be harder to push to every user) or a breaking minor version increment (in what case we do want the breaking change process to ensure that we don't actually break anything on a minor version).

I'm fine with a minor version increment if stakeholders accept it. It might be breaking, but it takes real effort to write code that'd break.

Can't be just a patch version increment. it actually changes something that people might want to opt in to, so it should have a real release version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. breaking-change-request This tracks requests for feedback on breaking changes
Projects
Status: In review
Development

No branches or pull requests

4 participants