[flow analysis] Rework demotion and type of interest promotion. #4370
Conversation
This formulation is consisent with the behavior of the implementation. Fixes dart-lang/sdk#60646.
| denotes set subtraction)_. | ||
| - If the `written` type is in `p2`, then `promoted` is `[demoted, | ||
| written]`. _Writing a value whose static type is a type of interest promotes | ||
| to that type._ |
There was a problem hiding this comment.
This would then be the location where written is a type in p2, and it is not a subtype of theCurrentType. So this is about demotion to a type of interest.
I think we'd use demote([declared, ...demoted], written) (possibly renamed as demotionTypeCandidates([declared, ...demoted], written)) to find the list of types that are of interest and supertypes of T, which would then be the value of promoted.
There was a problem hiding this comment.
This would then be the location where
writtenis a type inp2, and it is not a subtype oftheCurrentType. So this is about demotion to a type of interest.
I don't have a notion of "demotion to a type of interest" in my head when I think of flow analysis. As I mentioned above, I think of assignment as a two-step process: demotion, followed by promotion to a type of interest. In some cases a user might think that what's happening is demotion to a type of interest, e.g.:
f(Object x) {
if (x is num) {} // `num` is now a type of interest
x as int; // `x` is promoted to `int`
x = 1.5; // `x` looks like it got "demoted" to `num`
}But what really happened was that x got demoted from int to Object and then promoted to type of interest num.
Note that there are plenty of cases where type of interest promotion happens without any demotion, e.g.:
f(int? x) { // `int` is a type of interest
if (x == null) {
x = 0; // `x` is promoted to `int`
}
}In this case, demote returns an empty promotion chain (since there was no promotion prior to the assignment), and then toi_promote forms a new promotion chain by appending int onto it.
I think we'd use
demote([declared, ...demoted], written)(possibly renamed asdemotionTypeCandidates([declared, ...demoted], written)) to find the list of types that are of interest and supertypes ofT, which would then be the value ofpromoted.
See my previous comment about why demote returns a promotion chain, not a set of type candidates.
| - Let `p2` be the set `p1 \ { currentType(declared, demoted) }` _(where `\` | ||
| denotes set subtraction)_. | ||
| - If the `written` type is in `p2`, then `promoted` is `[demoted, | ||
| written]`. _Writing a value whose static type is a type of interest promotes |
There was a problem hiding this comment.
I feel like this is equivalent to what I've said, except that my way of saying it (promoted is [...demoted, written]) is consistent with the fact that promotion chains are considered to be immutable.
| denotes set subtraction)_. | ||
| - If the `written` type is in `p2`, then `promoted` is `[demoted, | ||
| written]`. _Writing a value whose static type is a type of interest promotes | ||
| to that type._ |
There was a problem hiding this comment.
This would then be the location where
writtenis a type inp2, and it is not a subtype oftheCurrentType. So this is about demotion to a type of interest.
I don't have a notion of "demotion to a type of interest" in my head when I think of flow analysis. As I mentioned above, I think of assignment as a two-step process: demotion, followed by promotion to a type of interest. In some cases a user might think that what's happening is demotion to a type of interest, e.g.:
f(Object x) {
if (x is num) {} // `num` is now a type of interest
x as int; // `x` is promoted to `int`
x = 1.5; // `x` looks like it got "demoted" to `num`
}But what really happened was that x got demoted from int to Object and then promoted to type of interest num.
Note that there are plenty of cases where type of interest promotion happens without any demotion, e.g.:
f(int? x) { // `int` is a type of interest
if (x == null) {
x = 0; // `x` is promoted to `int`
}
}In this case, demote returns an empty promotion chain (since there was no promotion prior to the assignment), and then toi_promote forms a new promotion chain by appending int onto it.
I think we'd use
demote([declared, ...demoted], written)(possibly renamed asdemotionTypeCandidates([declared, ...demoted], written)) to find the list of types that are of interest and supertypes ofT, which would then be the value ofpromoted.
See my previous comment about why demote returns a promotion chain, not a set of type candidates.
There was a problem hiding this comment.
The assign operation will reduce the promotion chain by removing non-supertypes of written. Then it will add the best type of interest, iff there is a unique best one which is a supertype of written and a proper subtype of the last element in the reduced promotion chain. In any case, the new current type of the variable is the last element in [declared, promotionChain].
I think your informal description along these lines is very helpful.
However, I think the naming of various elements is confusing. I suggested a couple of changes (in particular demoted and current is confusing), but I think there may be a need to adjust even more names.
It is very much about local reasoning. If the requirements on each argument of the semantic functions are stated concisely and up front then those requirements can be relied upon when reading the rest of that definition (e.g., the definition of toi_promote). With the current style there is some post-hoc reasoning about why certain properties are guaranteed to hold, but that's too late because the reader needs to know these things already when reading from the beginning of the definition of each semantic function.
This is a really good point, thank you. I think my latest revisions should address most of these issues. (I'm still using the name |
|
I looked at the live threads and closed the ones that could be considered to be resolved (I hope I got that right). The result is that there are no open threads. |
Thanks Erik! Landing now. |
This formulation will be consisent with the behavior of the implementation once https://dart-review.googlesource.com/c/sdk/+/427920 lands.
Fixes dart-lang/sdk#60646.