-
Notifications
You must be signed in to change notification settings - Fork 29
#3292. Update assertions for promotion_via_assignment_*
tests
#3294
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
base: master
Are you sure you want to change the base?
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.
Looks good! I suggested one adjustment of a description. The two tests at the end are more tricky (they must have slipped under the radar back when they were introduced). I'm explaining why I don't think the first one is testing what it says, and suggesting that the last one could be removed if there's no obvious way to make it work well.
/// @description Checks that if the variable was assigned after it was made a | ||
/// type of interest then promotion via assignment is not performed. |
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 think the old description was better:
/// @description Checks that if the variable was assigned after it was made a | |
/// type of interest then promotion via assignment is not performed. | |
/// @description Checks that if the variable was assigned after it was write | |
/// captured then promotion via assignment is not performed. |
/// that the static type of `E1'` must be a subtype of `x`'s declared type. | ||
/// | ||
/// @description Checks that a variable is not promotable from the type `S` to | ||
/// the type `T` if `T` is not a subtype of `S`. Test extension types. |
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.
This is somewhat twisted, and I can't really see what S
and T
should stand for. ;-)
At first, o
has the declared type Object?
and Object
is a type of interest (based on the declaration alone). At the beginning of line 94, 100, 106, o
still has type Object?
because no evidence has been gathered for anything else, but ET1/2/3
is now a type of interest. Then we assign an int
to o
and promote o
to Object
because that is a type of interest and a supertype of int
.
The extension type is a type of interest, but it is not the assigned type and not a supertype of the assigned type, so it isn't a member of p3
. As far as I can see, p3
contains exactly one element, namely Object
. So there's no way the extension type could be a candidate for being the new promoted type of o
.
So S
could be Object?
and T
could be Object
, but this contradicts the fact that we do promote o
from Object?
to Object
. Otherwise S
could be Object?
and T
could be the extension type, but it doesn't make much sense to say that we don't promote o
to the extension type, because we aren't even trying: the assignment is the only action that could justify a promotion which is in place when the expectStaticType
is reached. (And if we did do o = ET1(42)
then the promotion would occur.)
However, I think the test could match the description if the assignments were adjusted to use a different extension type:
void test1(Object? o) {
if (o is ET1) {
o = ET2(42);
o.expectStaticType<Exactly<Object>>();
}
}
void test2(Object? o) {
if (o is ET2) {
o = ET3(42);
o.expectStaticType<Exactly<Object>>();
}
}
void test3(Object? o) {
if (o is ET3) {
o = ET1(42);
o.expectStaticType<Exactly<Object?>>();
}
}
With this, both S
and T
is an extension type, and we are actually trying to promote from one of them to the other, and they are not subtype-related. The outcome will then be Object
or Object?
, depending on whether or not we can know for sure that it isn't null.
/// that the static type of `E1'` must be a subtype of `x`'s declared type. | ||
/// | ||
/// @description Checks that a variable is not promotable from the type `S` to | ||
/// the type `T` if `T` is not a subtype of `S`. Test extension types. |
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.
We can't do exactly the same thing here because the extension types aren't subtypes of the declared type. We could adjust the extension types such that it isn't an error, or we could drop the test if it turns out to be to similar to the previous one.
Please note that this PR updates assertions and descriptions but also fixes
promotion_via_assignment_A03_t07.dart
. Now it passes and I believe this is according to the changed spec. Please review.