#3057. Add more flow analysis tests for type Never #3070
Conversation
| /// | ||
| /// @description Checks reachability after variable or getter. Test variable of | ||
| /// type Never | ||
| /// @description Checks that the code is unreachable after a variable of type |
There was a problem hiding this comment.
The important part is that the code is unreachable because an expression was evaluated, and the static type of this expression is Never (or a subtype thereof, e.g., X extends Y, Y extends Never).
There's no way we should test all kinds of expressions. I'm sure basically every kind of expression can have the static type Never. E.g., e is T has type bool no matter what, but if e has static type Never then e is T is also guaranteed to never complete with a value, it must throw, or it must loop forever.
So how do we proceed? I think it's fine to cover just one or two expressions whose type is Never, and a formal parameter with that declared type is fine.
However, it is much more important to detect that various forms of expressions and statements in the enclosing code have a correct flow analysis. So this means that code after e as Never and e is Never must be flagged as dead, and similarly for [e1, e2, e3] where at least one of e1..e3 has type Never.
This would perhaps go to some other tests, if these tests are specifically about expression statements. Or perhaps those cases are already covered by other tests?
There was a problem hiding this comment.
The next PR (#3071) adds such tests for collection literals. In general, I plan to add tests for all statements (where possible) in Expressions and then for statements listed in Statements. Therefore, for example, in #3071, I check control flow operators (I know there are no for-in loops; I will add them later) but do not check 'regular' for-loops. Such tests will be added as part of the tests for the 'for' statement.
|
|
||
| main() { | ||
| try { | ||
| test(throw "Lily was here"); |
There was a problem hiding this comment.
Probably better: Drop the try/catch and do
| test(throw "Lily was here"); | |
| test(null); |
| T get n => throw "Lily was here"; | ||
|
|
||
| test() { | ||
| try { |
There was a problem hiding this comment.
OK, we can't rule out T == Never in this case, so try/catch is justified.
|
|
||
| main() { | ||
| try { | ||
| test(throw "Lily was here"); |
There was a problem hiding this comment.
Again, test(null) would suffice.
|
|
||
| import 'dart:async'; | ||
|
|
||
| FutureOr<Never> get n => 2 > 1 ? throw 1: Future.value(throw 2); |
There was a problem hiding this comment.
Perhaps?:
| FutureOr<Never> get n => 2 > 1 ? throw 1: Future.value(throw 2); | |
| FutureOr<Never> get n => 1 > 2 ? throw 1: Future.value(throw 2); |
This would allow the invocation of n to complete normally, and then main wouldn't have to have the try/catch (which doesn't really seem to help testing anything in the description).
There was a problem hiding this comment.
But in this case n wil have value Future.value(throw 2) which throws. Other values (say Future.value(2) or Future.value(null) failing the type check (n has type FutureOr<Never>). So, seems that we should have try/catch in this test.
| /// | ||
| /// @description Checks reachability after variable or getter. Test variable of | ||
| /// type Never | ||
| /// @description Checks that the code is unreachable after a variable of type |
There was a problem hiding this comment.
The next PR (#3071) adds such tests for collection literals. In general, I plan to add tests for all statements (where possible) in Expressions and then for statements listed in Statements. Therefore, for example, in #3071, I check control flow operators (I know there are no for-in loops; I will add them later) but do not check 'regular' for-loops. Such tests will be added as part of the tests for the 'for' statement.
|
|
||
| main() { | ||
| try { | ||
| test(throw "Lily was here"); |
|
|
||
| main() { | ||
| try { | ||
| test(throw "Lily was here"); |
|
|
||
| import 'dart:async'; | ||
|
|
||
| FutureOr<Never> get n => 2 > 1 ? throw 1: Future.value(throw 2); |
There was a problem hiding this comment.
But in this case n wil have value Future.value(throw 2) which throws. Other values (say Future.value(2) or Future.value(null) failing the type check (n has type FutureOr<Never>). So, seems that we should have try/catch in this test.
No description provided.