Flow analysis could handle final variables with assignments better

[stereotype441]

Consider the following code (adapted from discussion in #1247):

bool f(bool b) {
  final num a; // (1)
  if (b) {
    a = 0; // (2)
  } else {
    a = 0.5; // (3)
  } // (4)
  bool Function() g;
  if (a is int) { // (5)
    g = () => a.isEven; // (6)
  } else {
    g = () => false;
  }
  return g();
}

This has an error at the line marked (6): The getter 'isEven' isn't defined for the type 'num'.

If the lines from (1) to (4) are replaced with:

  final num a = b ? 0 : 0.5;

then no error occurs.

What's happening is that flow analysis sees the assignments to a on lines (2) and (3), and conservatively assumes that they could happen at any time, including between the time of the test at (5) and the usage at (6). So it disables promotion. But this is nonsense, because a is a final variable, therefore at the time of the test on line (5), it is guaranteed to be already assigned (otherwise line (5) would have had an error), and thus no further assignments to it can occur. So it should be safe to promote.

It should be safe to alter flow analysis so that assignments do not defeat promotion if the variable assigned to is final. This will work even for late final variables, because such variables are still guaranteed not to be assigned to more than once (the only difference is that the checking is done at runtime rather than statically).

[stereotype441]

@renatoathaydes FYI

[srawlins]

Related to, maybe identical, #1536

[stereotype441]

Related to, maybe identical, #1536

Good point, @srawlins. IMHO they're not identical because #1536 is about a variable that's definitely not final. But they're definitely related bugs. Hopefully we can think of a fix that would address both of them.

[renatoathaydes]

@stereotype441 the if in the assignment is not necessary, this simplified version of the code still won't compile:

bool f(bool b) {
  final num a; // (1)
  a = 2;
  bool Function() g;
  if (a is int) { // (5)
    g = () => a.isEven; // (6)
  } else {
    g = () => false;
  }
  return g();
}

[Levi-Lesches]

Well as long as we're playing code golf, I'll join in 😁

void f(bool b) {
  final num a;
  a = 2;
  if (a is int) {
    final g = () => a.isEven; // error: isEven isn't defined for num
  }
}

[stereotype441]

Well as long as we're playing code golf, I'll join in 😁

@Levi-Lesches Haha, challenge accepted! How about:

f(){final Object a;a=2;a is int?()=>-a:0;}

More seriously, @renatoathaydes is right, the if in the assignment is not an essential part of this bug 😃

[renatoathaydes]

Related to, maybe identical, #1536

That issue shows that people expect even non-final local variables to work in such cases because that's safe (if the variable is not re-assigned anywhere). I wrote a comment in that issue showing how that works in Kotlin (basically using the concept from Java of effectively final).

@Levi-Lesches exactly, even the other if was unecessary. But the point was that Dart behaves particularly badly because it distinguishes between:

final num a;
a = 1;

and

final num a = 1;

[eernstg]

It's an even better point if we can treat

  final num a;
  if (b) {
    ... // Do arbitrary stuff, just nothing about `a`.
    a = 0;
    ... // Do arbitrary stuff, just nothing about `a`.
  } else {
    ... // Do arbitrary stuff, just nothing about `a`.
    a = 0.5;
    ... // Do arbitrary stuff, just nothing about `a`.
  }

and

  final num a = b ? 0 : 0.5;

as equivalent, with respect to promotions of a.

[kallentu]

dart-lang/sdk@47d5502 addresses this behaviour (although, currently under the inference-update-4 flag). Closing.