Inconsistent behavior of field promotion in irrefutable patterns.

[stereotype441]

During the development of the patterns feature, we decided that refutable pattern matches should trigger type promotion of the scrutinee, but irrefutable pattern matches shouldn't. For example, this is allowed:

test(int? i) {
  switch (i) {
    case var x!:
      i.isEven; // OK; `i` was promoted to `int` by the pattern match.
  }
}

But this is a compile-time error:

test(int? i) {
  var (x!) = i;
  i.isEven; // ERROR; `i` must be null checked.
}

I just realized that when I implemented field promotion, I forgot to follow these rules. Currently, field promotion works both in refutable and irrefutable pattern matches. For example:

class C {
  final int? _i;
  C(this._i);
}

test(C c) {
  var (x!) = c._i;
  c._i.isEven; // OK; `c._i` was promoted to `int` by the pattern match.
}

If I had discovered this problem earlier, I would have just fixed it without raising a fuss. But since we've passed the branch cut deadline for 3.2, I wanted to get the language team's opinion about what to do. Should I:

1. Try to fix the inconsistency ASAP (so that the last example above is a compile-time error) and cherry-pick the fix into 3.2? This option is the best from the standpoint of language consistency, but it carries some non-zero risk of endangering the stability of the Dart 3.2 release, which is less than a month away (November 15).
2. Fix the inconsistency in Dart 3.3, as a breaking change? This option avoids any risk to the stability of Dart 3.2, but it has the disadvantage that Dart 3.2 could in principle break some user code. I could do some experiments in Google3 to try to estimate just how breaking this would be.
3. Fix the inconsistency for Dart 3.3, as a language-versioned change? This option is even better from the standpoint of risk, since it ensures that user code will only be broken by the change when they change their language version. But it's more costly to implement, since it means that flow analysis will have to support both the new and old behaviors for a long time.
4. Don't bother fixing the inconsistency at all. This option is both low risk and low effort, but it leaves us with a rather unfortunate inconsistency that we have to document somewhere.

Personally I lean towards either 1 or 4, but I'm curious what others think.

CC @dart-lang/language-team

[munificent]

we decided that refutable pattern matches should trigger type promotion of the scrutinee, but irrefutable pattern matches shouldn't.

For the record, the discussion of this is here and the rationale is, I think this comment.

I lean towards 1, but I don't have much visibility into how safe cherrypicks are for this release.

Barring that, I think we could probably do 2. Since pattern variable declarations and assignments generally require the assigned value to already be a subtype of the pattern's type, I don't think the rule will come into play very often. You have to use a cast pattern, ! pattern or, (maybe?) have a value of type dynamic. I suspect those are all pretty rare.

If we end up doing 4, I won't lose sleep over it.

[lrhn]

We don't promote on an implicit downcast from dynamic, so assignment in a declaration pattern shouldn't promote the assignee.

On the other hand, an ! or as int, which we do allow in non-refutable patterns, should probably promote, even inside a declaration or assignment pattern.
I think it was a mistake that we forgot that originally, not an intended behavior.

There is no type check in a declaration pattern dure to a binding, object, record, list or map pattern, like there would be in a refutable pattern, but an embedded ! or as should still count, because they act the same in all patterns.
Those are not assignments, statically checked in a declaration pattern, they are real runtime type tests.

The original argument for not doing promotion in a declaration pattern was probably not considering those at all, only the patterns that actually differ in behavior between refutable and non-refutable patterns.

[stereotype441]

Hmm, good thoughts! Building on what @lrhn said, I guess we have a fifth option available:

5. Leave field promotion behavior as is for Dart 3.2. In a future language version, change the behavior of both field promotion and local variable promotion so that null-assert and cast patterns always promote (regardless of whether they're in a refutable or irrefutable context), but dynamic downcasts in an irrefutable context don't promote.

The change from today's behavior to that future behavior would then be:

• null-assert and cast patterns begin promoting local variables in irrefutable contexts (currently they only promote fields in refutable contexts).
• dynamic downcasts stop promoting fields in irrefutable contexts.

And maybe an option 6:

6. Same as 5, but do it as a breaking change rather than a new language version.