#1401. Calculating the static type of the pattern tests added #1847
Conversation
There was a problem hiding this comment.
I did not look at the last 6 or so tests, because they basically all need to have explicit type arguments on the initializing expressions, and that will determine what gets filled in at all the locations in the pattern where a type is missing.
In general, this makes the tests more predictable because we're testing what the pattern will do with a certain matched value type, so it's simpler if that matched value type is completely explicit and unambiguous.
There was a problem hiding this comment.
Tests updated according to the review. Main changes are:
- Type arguments added on right hand site
- Tests split into 3 parts
a. Testing filling "holes" from initializing expression
b. Test int-to-double conversion
c. Testing casts fromdynamicand generic function instantiation
There was a problem hiding this comment.
Lots of fixes noted, thanks!
However, there are still several locations where a test has an initializing expression whose typing properties aren't settled up front (that is, some type arguments or parameter types or whatever it is are computed based on the context type schema from the pattern).
It is my understanding that this entire PR is concerned with phase 3, and I believe that phase is tested most precisely if phase 1 and phase 2 are a no-op, that is: The type schema of the pattern isn't used, and no type inference takes place on the initializing expression.
It is true that some parts of phase 3 are about propagating information from some parts of the pattern to other parts (so, e.g., a list pattern like <int>[v1] would propagate the information that the list has element type int to the element v1, whose declared type is then also int).
This does mean that there are cases where the fully inferred type of the initializing expression is sort of unused, because the pattern already says something about the type arguments of a list or map pattern.
But it is still nice to have a completely firm initializing expression, such that we know exactly which two types are involved in the final assignability check.
| x2.expectStaticType<Exactly<double>>(); | ||
| Expect.identical(42.0, x2); | ||
|
|
||
| var [x3, ...List<double> r2] = [1, 2, 3]; |
There was a problem hiding this comment.
Fixed. Please note var <double>[x1] = [42]; is an error on dartk but not an issue in analyzer. Analyzer issue?
| x2.expectStaticType<Exactly<double>>(); | ||
| Expect.identical(42.0, x2); | ||
|
|
||
| var [x3, ...List<double> r2] = [1, 2, 3]; |
There was a problem hiding this comment.
var <double>[x1] = [42]; should work just fine: It would yield a pattern type schema of List<double> which would turn [42] into <double>[42.0]. I don't see any errors (compile-time or run-time), so what did dartk complain about?
| import "../../Utils/expect.dart"; | ||
|
|
||
| main() { | ||
| var <double>[x1] = [42]; |
There was a problem hiding this comment.
Analyzer Ok. Dartk:
stderr:
tests/co19/src/LanguageFeatures/Patterns/type_inference_A17_t05.dart:21:15: Error: The matched value of type 'List<int>' isn't assignable to the required type 'List<double>'.
- 'List' is from 'dart:core'.
Try changing the required type of the pattern, or the matched value type.
var <double>[x1] = [42];
^
There was a problem hiding this comment.
That would be a CFE issue: The type schema from the pattern is List<double> and [42] inferred with context type List<double> should yield <double>[42.0]. So keep it.
|
This Github UI won't let me respond to the comment about LanguageFeatures/Patterns/type_inference_A16_t03.dart, line 34, so here we go:
Yes (a CFE issue, presumably). I expect that the CFE will start generating code for the generic function instantiation soon, so right now we should just keep the code and make sure there is an issue and then approve the test failure (if any) with a reference to that issue. |
| import "../../Utils/expect.dart"; | ||
|
|
||
| main() { | ||
| var <double>[x1] = [42]; |
There was a problem hiding this comment.
That would be a CFE issue: The type schema from the pattern is List<double> and [42] inferred with context type List<double> should yield <double>[42.0]. So keep it.
2023-02-17 [email protected] dart-lang/co19#1399. [Records] Allow legacy libraries that don't support records to still be able to see the Record class in "dart:core" (dart-lang/co19#1571) 2023-02-17 [email protected] dart-lang/co19#1860. Fix type_inference_A12_t01.dart. Don't expect warning (dart-lang/co19#1862) 2023-02-17 [email protected] Fixes dart-lang/co19#1860. Fix roll failures (dart-lang/co19#1861) 2023-02-16 [email protected] dart-lang/co19#1401. Calculating the static type of the pattern tests added (dart-lang/co19#1847) 2023-02-16 [email protected] dart-lang/co19#1401. Shared case scope test with labels and default case added (dart-lang/co19#1858) 2023-02-15 [email protected] Fixes dart-lang/co19#1837. Add additional unnecessary null-ckeck warnings (dart-lang/co19#1842) Change-Id: I3a1f5deafd4f2868574f085495417ed37fe845b9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284080 Reviewed-by: Alexander Thomas <[email protected]>
No description provided.