-
Notifications
You must be signed in to change notification settings - Fork 213
Type promotion in pattern &&
#2622
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
Yes, I'd like this to work too. Required type isn't the right property we want to use because the required type of cast and assert patterns is deliberately wide to allow them to be used in variable declarations. But I talked about this a bit here and it should be fairly straightforward for us to define a "demonstrated type" for each pattern that would work here. Another way we could think of it as that we could treat the value flowing into an There may be cases where that produces better results than just specifying that we do GLB on the matched value type and LHS's demonstrated type. |
+1 to this. I actually defined a notion of "demonstrated type" in my proposal for how to apply flow analysis to patterns (circulated internally but not yet published externally), so we're definitely on the same wavelength there. Here's how I defined "demonstrated type" from that doc:
(With the caveat that LUB and GLB have some surprising counterintuitive behaviour so we might want to do something different for logical-or and logical-and patterns). Note all the caveats above that say "if it’s a subtype of the matched value type. Otherwise the matched value type." These establish the invariant that demonstrated type is always a subtype of matched value type, which is in alignment with how we handle promotion in ordinary
I'm wary about this for reasons I'm having trouble putting my finger on. Maybe it's because flow analysis is built to model imperative logic, and patterns are about as non-imperative as Dart gets. I'd be more inclined to go with something more along the lines of what @lrhn proposed, but rephrasing it slightly to make use of "demonstrated type", and dropping the GLBs (which become unnecessary due to the invariant I mentioned above):
|
All that sounds right to me. |
This is broadly in line with my thoughts as I read through the various discussions. My guess is that GLB probably does the wrong thing more often than the right thing. The interesting case for I'm not sure how we associate the |
Thinking about this in the context of #2620, I think it's worth also thinking about this for |
A couple more comments. First, defining the failure type for or patterns also gives a framework for thinking about improving the matched value type in successive cases. That is, I would expect that Second, a note on the demonstrated type. I think for constants, we can do slightly better than just preserving the matched value type. That is, I think the constant pattern Lastly, the flow analysis sketch here defines an operation called |
Yes, and there's already a natural mechanism in the proposal to hang this behavior off of: invocation keys. We already specify that any series of invocations is cached and subsequent "calls" to that same series of invocations uses the previously cached value. That way we can soundly do exhaustiveness checks in the presence of unknown types that could mutate under the hood. The proposal even sketches out an analogy for how that caching works by desugaring a pattern to a series of late local variables: main() {
var list = [1, 2];
late final $match = list;
late final $match_length = $match.length;
late final $match_length_eq2 = $match_length == 2;
late final $match_0 = $match[0];
late final $match_1 = $match[1];
late final $match_0_eq1 = $match_0 == 1;
late final $match_1_lt4 = $match_1 < 4;
late final $match_0_isEven = $match_1.isEven;
late final $match_0_isEven_eqtrue = $match_0_isEven == true;
if ($match_length_eq2 &&
$match_0_eq1 &&
$match_length_eq2 &&
$match_1_lt4) {
print('first');
} else if ($match_length_eq2 &&
$match_0_isEven_eqtrue) {
var a = $match_1;
print('second $a');
}
} We could track whenever a pattern shows that the result of some invocation key has or does not have some type. Later patterns that access the same invocation key could then potentially see the value with a properly promoted type. Doing this right gets pretty subtle, though. Within the same case, we should be able to safely promote later accesses of the same invocation key to any type shown by previous uses of that invocation key in that case. We know we can only get to the later ones if the previous ones matched. Across cases is harder. This should be valid: switch (3 as int?) {
case null: print('null');
case var x: print(x.isEven);
} But this isn't: switch ((3 as int?, 4)) {
case (null, 5): print('null');
case (var x, _): print(x.isEven);
} Because here we only know that the previous case failed if we reach Even so, I do think it makes sense to do positive promotion within a single case, and I think invocation keys might be the right thing to hang that off of. |
My only concern here is that it's a bunch of additional flow analysis work. Maybe that's a good thing though--a deep understanding of how flow analysis works is mostly siloed in my brain right now, and work like this might be a good excuse for someone else to become more familiar with it.
I'm actually less concerned about this. This is precisely the sort of thing that flow analysis already does really well, and I think it will just fall out of the algorithm. |
@stereotype441, my understanding is that the type promotion stuff you're doing for patterns does handle this. Is there anything I need to do in the proposal for this? Can I (or you if you want the satisfaction) close the issue? |
The current type inference specification for patterns use the same mathed value type for both branches of an and-pattern.
That effectively means that the and-pattern cannot promote the matched value from the first clause to the second, and use the promoted type in the second clause.
Examples where that might be valuable could be:
The former can be written as
(>0)?
, because we have an inline null-check, the latter cannot as easily.Users will likely expect promotion for patterns like this.
Maybe it's as easy as saying:
(Don't know if the
or
pattern needs work too, butcase == null | <= 0
on anint?
might be useful as well. Can't be expressed as simply as the above, so we might want to use our entire type promotion machinery, and treat the matched value as a variable. Which it is, since we cache the value it's evenfinal
.)The workaround, which I never want to see happen:
Removing any incentive for that workaround is motivation enough to do something!
The text was updated successfully, but these errors were encountered: