-
Notifications
You must be signed in to change notification settings - Fork 213
Should an is T
test make T?
a type of interest?
#4361
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
Comments
The reason why we don't always give a promotable variable which is being assigned a new value the type of the assigned expression (except that we wouldn't ever do it for The reason why Hence, making We could certainly do it, and it seems useful in the example, but it is at least a lot more marginal than the direction "T? -> T" that we already have. |
The reason we don't always promote is indeed that it may be confusing when the promoted type gets captured. num x;
// ...
if (case1) {
x = 0.0; /
// something something
if (case1a) {
var finalValue = 37;
x = 42;
return [x]..add(finalValue);
}
// ... If That's a risk. If we promote and the user doesn't expect it, they get subtle changes in behavior from what they expect. So, assignment only promotes if the user has shown some interest in whether the variable has that particular type. Nullability is not like a type, it's more of an "either something of that type or not". int toInt(Object? o) {
if (o == null) return 0;
if (o is! int) {
if (o is double) {
o = o.toInt();
} else {
o = int.tryParse("$o");
}
}
return o ?? -1;
} I feel like the
The variable doesn't have to be promoted to the type of interest. It's also of interest on the non-promoted branch. That's actually where it's usually useful: Basically: Special-case nullability, treat it as not a type, but as a variation. All variations of (When we intersect promotion chains, do we recognize fx |
If I understand this correctly, I think this would introduce a new situation in which we can promote a variable to a type which is not a subtype of its original declared type. There's nothing per se wrong with that, but it's a bit odd, and I'm not sure I love it. I think it means that you can end up in a situation where you have to demote the variable to something more general than its original declared type: void test1(Object o) {
if (o is int) { // int? is a type of interest now
o = o == 3 ? null : 0;
}
// What is the type of o on the join? Do we use UP to find a common super type?
o = null; // valid because o has type Object? now?
} In general, this feels like kind of a tarpit to me. Is this right, or do the above complications not arise for some reason? |
Related discusion here, for reference. Also, I'm pretty sure that at some point we considered a generalization of this in which an assignment of something of type g(Object n) {
if (n is! int) return;
// `n` is promoted to `int` now
n = 3.4
// UP(double int) is a type of interest, therefore `n` is now promoted to `num`.
} |
Agreed that would be a tarpit. Fortunately there's a subtlety in the "promote to type of interest" logic that prevents this kind of complication from arising; unfortunately that subtlety is not documented in flow-analysis.md. I'm going to try to make a PR to rectify that this week (gosh, is it Thursday already?), but for now you can look at my informal description from issue 60646. In short, the "promote to type of interest" logic first cancels any promotions that are no longer valid; then, when considering candidate types of interest that it might promote to, it filters out any candidate types that are not a subtype of the current type. This preserves the invariant (also maintained by other parts of flow analysis) that every type in the chain of promotions is a subtype of the previous ones (and the first type in the chain of promotions is a subtype of the declared type). Thus, by transitivity, the promoted type is always a subtype of the declared type. So in your example above, even if we say that |
I couldn't entirely follow the paragraph that started with "in short", because I'm not sure entirely what exactly things like "promotions that are no longer valid", "candidate types", "current type" etc mean. Sorry, I don't know how much of this is me not being super dialed in on this vs the paragraph just being informal. :). Working from the paragraph quoted above though, I don't think the logic there is enough (which is not to say that the full logic doesn't handle it. Expanding my example: void test1(Object? o) {
if (o is Object) {
if (o is int) { // int? is a type of interest now
o = o == 3 ? null : 0;
}
// What is the type of o on the join? Do we use UP to find a common super type? Or demote to Object?
o = null; // valid because o has type Object? now?
}
} Here, the logic " |
Whether we filter types of interest at the type-check point or the assignment point, we should never actually use a type of interest which is not a subtype of the declared type of a variable. We can say that It's not new to have potential types of interest that are not subtypes of the declared type of the variable. void foo(Object o) {
if (o is! int?) { // Makes `int?` and `int` potentially types of interest, but `int?` isn't valid.
o = 42; // OK, promotes to int.
o = (42 as int?); // Not OK.
} About: g(Object n) {
if (n is! int) return;
// `n` is promoted to `int` now
n = 3.4
// UP(double int) is a type of interest, therefore `n` is now promoted to `num`.
} That would make every supertype of void somethingWidget(Widget widget) {
if (widget is ClipRect) return; // Don't do anything, they're special.
widget = Transform(child: widget, ...);
// What is the type of `widget` here? The type is SingleChildRenderObjectWidget, which both Is that a problem? Probably usually not, but the reason we don't just assignment promote every assignment also applies here. var listOfWidget = [widget];
workWithListOfWidget(listOfWidget); // Error. Cannot add MultiChildRenderObjectWidget to list. I think nullability is special, and it's OK to special case it. I don't think it necessarily generalizes. |
At the site of Then, promotion to types of interest is considered. Today, the types of interest are At the end of the At the point of the assignment If we accept the suggestion that a test on At the site of But then, when promotion to types of interest is considered, the types of interest will be Then, at the end of the |
@stereotype441 thanks, that helps. Stepping up a level, I don't really have strong feelings here, but I admit to having a hard time formulating a crisp story for why we should do this. I don't find anything here particularly compelling either way. I can certainly see cases where it is useful to promote to the nullable version of a type of interest. I can also see cases where it is useful to promote to a different super type of a type of interest (e.g. promote to num given that double is a type of interest). Taken to the limit, we could just say that a variable has the type of whatever the last thing that was assigned to it was. Where do we draw the line and why? And if we're going to move that line, what's the principled reason for this particular choice of how to do so? |
My reasoning here is that nullability is different from other supertypea and subtypes. We already accepted that when we made NonNull(T) a type of interest when you check for That logic also applies when you start out asking |
Agreed! This is addressed in #4370 (spec) and https://dart-review.googlesource.com/c/sdk/+/427920 (implementation).
I agree that, as an implementation optimization, it makes sense not to bother remembering a type of interest that's never going to have an effect. As for what we say in the spec, what I found easiest to do in #4370 was to say that each variable has a set of
I agree. One of the design principles I remember coming up in the early discussions about flow analysis was that we wanted to make sure we didn't promote to types that would be "surprising" to users. That was where the idea of "types of interest" came from. I feel like if we made UP(written_type, tested_type) a type of interest, we would be creeping too far into "surprising" territory. |
FWIW, I think @lrhn has a point that if we're going to make a NonNull(T) a type of interest when you check for T, it makes sense to also make T? a type of interest. That to me is enough to count as a principled reason. But so far I haven't seen a compelling real-world example where it matters (i.e., some code in the wild where someone is doing a type test of T, and later an assignment of T?, and then having to do extra work because a type-of-interest promotion didn't occur). I would probably want to dig around google3 looking for some concrete examples before I dove into working on this. |
I don't really buy this, FWIW. I can see an argument for saying that
Yeah, this would be a lot more compelling of an argument if we had some actual use cases to work from. |
I think nullability is different from other union types. Well, the other union type. And maybe it isn't really, but it's just more common. If I do We don't say something similar for If you have: For nullability, we don't make Of the two union types and two component types, the one that stands out here is |
Currently flow analysis has the rule that an
is T
oras T
test causes bothT
and NonNull(T
) to become types of interest. This allows the user to do things like this:If we didn't consider NonNull(
T
) to be a type of interest, then the statementn = 0
would merely promoten
toint?
.In dart-lang/sdk#60646 (comment), @lrhn suggested that maybe an
is T
test should also makeT?
a type of interest. This would allow things like this:(Today,
int?
is not a type of interest, so the assignment ton
ing
loses the promotion completely, andn
gets demoted back tonum?
).CC @dart-lang/language-team
The text was updated successfully, but these errors were encountered: