dart-lang / dart-lang/language
Promotion of variables after assignment in try
- Dominant language
- TeX
- Stars
- 2.9k
- Forks
- 239
- Avg merge
- 2d 18h
- Merged PRs (30d)
- 14
Description
Take the following code:
```dart
import 'dart:math';
void main() {
String? s1;
String? s2;
Random r = new Random();
bool b1 = r.nextBool();
bool b2 = r.nextBool();
if (b1) {
s1 = giveMeString();
s2 = giveMeString();
}
if (s1 == null) {
print("s1 was null");
return;
}
if (s2 == null) {
print("s2 was null");
return;
}
// s1 and s2 are now non-null.
onlyAcceptsString(s1);
onlyAcceptsString(s2);
try {
// s1 = giveMeMaybeString(b2);
s1 = giveMeStringOrCrash(b2);
} catch (e) {
print("Crashed!");
}
if (!b2) {
s2 = giveMeStringOrCrash(b2);
}
// Now s1 is nullable.
onlyAcceptsString(s1);
onlyAcceptsString(s2);
}
void onlyAcceptsString(String s) {
print("Happy!");
}
String giveMeString() {
return "42!";
}
String? giveMeMaybeString(bool giveString) {
if (giveString) return "42!";
return null;
}
String giveMeStringOrCrash(bool crash) {
if (crash) {
throw "booh!";
}
return "42!!";
}
```
I'm puzzled by `s1` being nullable after the try. Either it's assigned a `String` and thus still a `String` or it isn't and it's also still a `String`. I understand that if I had uncommented the first line in the try and it had been
```dart
s1 = giveMeMaybeString(b2);
s1 = giveMeStringOrCrash(b2);
```
it could be null, but without it it can't.
Digging around in the flow analysis I see that in `tryCatchStatement_bodyEnd` it does a `conservativeJoin`, basically removing all promotion for written variables:
https://github.com/dart-lang/sdk/blob/c7625fc68dfb829d7ab9a3a6cbf50f069ad2cc84/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart#L4084-L4098
I've hacked around and noticed there's a `nonPromotionHistory` available. Utilizing that I've applied the following hack:
```dart
// Hack: Remove written variables that wasn't ever demoted.
Set written = {};
for (Variable v in info._written) {
VariableModel vModel =
afterBody.infoFor(info._written.first);
if (vModel.nonPromotionHistory != null) {
written.add(v);
}
}
FlowModel beforeCatch =
beforeBody.conservativeJoin(written, info._captured);
```
which seems to do what I'd want: Accept
```dart
// s1 = giveMeMaybeString(b2);
s1 = giveMeStringOrCrash(b2);
```
as non-nullable and mark
```dart
s1 = giveMeMaybeString(b2);
s1 = giveMeStringOrCrash(b2);
```
as nullable.
It also still demotes for instance
```dart
Object? foo;
foo = "Hello";
if (foo is String) {
try {
foo = 42;
} catch (e) {
print("Crashed!");
}
onlyAcceptsString(foo);
}
```
but is happy about
```dart
Object? foo;
foo = "Hello";
if (foo is String) {
try {
foo = "still string";
} catch (e) {
print("Crashed!");
}
onlyAcceptsString(foo);
}
```
Can we have something like this? Or is there some reason why this wouldn't work?
/cc @stereotype441 @lrhn @johnniwinther
Contributor guide
Research direction
Start in pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart at tryCatchStatement_bodyEnd and read how conservativeJoin uses written variables. Compare the issue's s1 and foo examples, including nonPromotionHistory, and determine the intended promotion behavior before validating it against the existing flow-analysis logic.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- dart
- Domain
- compilers
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100