Flow analysis feature: Assignment Promotion

[stereotype441]

(Note: this feature request was raised in #1420 under the heading "Feature: Assignment Promotion", but since it's a flow analysis request, and that issue is tagged "field-promotion", I'm splitting the feature request out to its own separate issue).

It would be nice to allow a local variable assignment of the form id assignmentOp expression (potentially parenthesized) to act like the variable itself if used in a test. We currently allow if (x != null) ... to promote x. This change would also allow if ((x = something) != null) ... to promote x.

It only affects the left-most variable of an assignment, so if ((x = y = something) != null) ... will only promote x, not both x and y (although that's also an option if we really want it - treat both x and the assigned expression as being tested)

If you do if ((x += 1) != null) ... that still works, but there aren't that many operators which return a nullable result, so the usefulness is limited.

This is a very small feature, but it allows you to do promotion by:

int? c;
if ((c = this.capacity) != null) { ... use(c)... }

[mateusfccp]

Seems a lot like #1201.

[lrhn]

If-variables (#1201) introduces a new variable. This limited feature would not introduce any new syntax, it would just allow promoting in more cases than we currently do.

(Promoting where we didn't before is a potentially breaking change, so we'd still need to introduce it in a new language version and with proper warning about things that might break.)

[mateusfccp]

I didn't notice that this syntax is already valid today... That does make sense, as it's less work.

[eernstg]

We should keep in mind that the following rewrite is available today:

void main() {
  {
    // With the feature we can do this:
    int? c;
    if ((c = this.capacity) != null) { ... use(c)... }
  }

  {
    // But we can do this already today:
    int? c = this.capacity;
    if (c != null) { ... use(c)... }
  }
}

So how much of an improvement is the new feature? I guess it could be an improvement with a long if-else chain:

void main() {
  int? c;
  if ((c = exp1) != null) { ... use(c) ... }
  else if ((c = exp2) != null) { ... use-c-in-some-other-way ... }
  ...
  else { c = some-non-null-expression; }
  // At this point we know that `c` is non-null.
}

This if-else chain isn't as easy to rewrite into something which is similarly concise without the feature, because we would have to put an assignment before each if. We'd probably use the following, but that is a bit more error prone and inconvenient:

void main() {
  int? c;
  L: {
    c = exp1;
    if (c != null) { ... use(c) ...; break L; }
    c = exp2;
    if (c != null) { ... use-c-in-some-other-way ...; break L; }
    ...
    c = some-non-null-expression;
  }
  // At this point we know that `c` is non-null.
}

Does someone have an example where this kind of promotion really shines?

[lrhn]

I'd go with loops.

Node lastInChain(Node node) {
  Node? next;
  while ((next = node.next) != null) {
    node = next;
  }
  return node;
}

Or nested expressions.

Node? next;
if (node != null && (next = node.next) != null) {
   actBetween(node, next);
}

[eernstg]

But also (I know, dart format sez no ;-):

Node lastInChain(Node node) {
  while (true) {
    var next = node.next;
    if (next != null) node = next; else break;
  }
  return node;  
}

[lrhn]

Dart format always win in the end. That, and reviewers not being used to labeled breaks.

(That's why I say:

do {
  var next = node.next;
} while (next != null) {
  node = next;
}

in #171 - a combined do-and-while loop with a shared scope).