-
Notifications
You must be signed in to change notification settings - Fork 28
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
#2080. Add correct member overrides tests. Part 2. Setters #2093
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a question.
/// is a correct override of m′ iff the following criteria are all satisfied: | ||
/// ... | ||
/// • If m and m′ are both methods or both setters: Let F be the function type | ||
/// of m except that the parameter type is the built-in class Object for each |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect to see Object?
instead of Object
in this line. Could you point me to the part of the spec this quote is taken from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it's a comment, it can be resolved in a follow-up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! That looks interesting :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With null safety, that should be Object?
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think it should be Object?
too, but the spec does say Object
. Should we update the spec as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spec updates for null safety are in review (...since september 2021...). It is in the PR dart-lang/language#2605: It was missing, but I just corrected that. So it will be landed along with all the other null safety changes.
2023-06-23 [email protected] Fixes dart-lang/co19#2080. Fix typo in correct_member_overrides_A03_t14.dart (dart-lang/co19#2097) 2023-06-22 [email protected] dart-lang/co19#2080. Move `Correct member overrides` to Superinterfaces folder (dart-lang/co19#2096) 2023-06-22 [email protected] Fixes dart-lang/co19#2083. Expect `unspecified` error in const_collections_A08_t01.dart (dart-lang/co19#2084) 2023-06-22 [email protected] dart-lang/co19#2080. Update assertion texts, add mixin syntax test (dart-lang/co19#2095) 2023-06-22 [email protected] dart-lang/co19#2080. Add correct member overrides tests. Part 2. Setters (dart-lang/co19#2093) 2023-06-21 [email protected] Fixes dart-lang/co19#2090. Add required final instance variable to a inline class (dart-lang/co19#2091) 2023-06-20 [email protected] dart-lang/co19#2080. Add correct member overrides tests. Part 1. (dart-lang/co19#2088) 2023-06-20 [email protected] dart-lang/co19#2080. Update assertion texts of overriding tests (dart-lang/co19#2089) 2023-06-15 [email protected] dart-lang/co19#1400. Add inline classes members override test (dart-lang/co19#2085) Change-Id: I4a5a112cb11590b0c15032a65f5b80193a42b27b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311160 Commit-Queue: Alexander Thomas <[email protected]> Reviewed-by: Alexander Thomas <[email protected]>
No description provided.