language icon indicating copy to clipboard operation
language copied to clipboard

Field promotions should apply inside irrefutable patterns.

Open stereotype441 opened this issue 7 months ago • 9 comments

During the implementation of patterns, we decided that irrefutable patterns shouldn't cause type promotions to happen. For example, this promotes d from type dynamic to int:

main() {
  dynamic d = 0;
  if (d case int x) {
    d; // Static type: `int`
  }
}

but this doesn't:

main() {
  dynamic d = 0;
  var (int x) = d;
  d; // Static type: `dynamic`
}

The reason we decided this was primarily so that pattern assignments and pattern variable declarations would behave consistently with ordinary assignments and variable declarations (see past discussion).

Unfortunately, the way I implemented this essentially caused all promotion related to the scrutinee of the irrefutable pattern match to be disabled, including field promotions. So, for example, if a field is promoted before the irrefutable pattern match, that promotion is "forgotten" during the irrefutable pattern match:

class A {
  final int? _i;
  A(this._i);
}
main() {
  var a = A(0);
  a._i!; // Promotes `a._i` to `int`
  var A(_i: i) = a; // Promotion is forgotten, therefore:
  int j = i; // ERROR: `int?` not assignable to `int`
}

But it all works works fine if the pattern match is refutable:

// Same definition of `class A`
main() {
  var a = A(0);
  a._i!; // Promotes `a._i` to `int`
  if (a case A(_i: var i)) {
    int j = i; // OK: `i` has type `int`.
  }
}

Similarly, a field promotion performed by ! or as on the inside of the left hand side of an irrefutable pattern match doesn't have any effect on the code that follows:

// Same definition of `class A`
main() {
  var a1 = A(0);
  var A(_i: i1!) = a1;
  int j = a1._i; // ERROR: `int?` not assignable to `int`
  var a2 = A(0);
  var A(_i: i2 as int) = a2;
  int k = a2._i; // ERROR: `int?` not assignable to `int`
}

Whereas these same promotions do have an effect after a refutable pattern match:

// Same definition of `class A`
main() {
  var a1 = A(0);
  if (a1 case A(_i: _!)) {}
  int j = a1._i; // OK: `a1._i` has been promoted to `int`
  var a2 = A(0);
  if (a2 case A(_i: _ as int)) {}
  int k = a2._i; // OK: `a2._i` has been promoted to `int`
}

This feels weird and surprising to me, and it feels like it goes well beyond the original intent.

I'm tempted to change the rules so that irrefutable pattern matches handle field promotion in the same way as refutable pattern matches, and the only difference in promotion between irrefutable and refutable patterns is that an irrefutable pattern match never promotes the type of the scrutinee itself. I think this would be much more in keeping with the original intent, and less confusing to users.

(See https://github.com/dart-lang/language/issues/4344#issuecomment-2845553237 for further context.)

@dart-lang/language-team What do you think?

stereotype441 avatar May 01 '25 21:05 stereotype441

As always, other things being equal, making the two kinds of pattern match more consistent is a good thing. It also seems useful to obtain more precise types on the variables that are now going to be promoted.

However, I'm not quite sure why we need this:

an irrefutable pattern match never promotes the type of the scrutinee itself.

If the irrefutable pattern match occurs as the behavior of a pattern variable declaration like var P = e; then the static type of e should be dynamic, or it should justify the viability of the match: If P is an object pattern built on a class C then the static type of e must be assignable to C; if P is a record pattern with specific required types for each component then the static type of e must be a record with the same shape and assignable component types, and so on. Technically, the requirement is that the static type of the initializing expression must be assignable to the required type of the pattern.

I'd expect this requirement to ensure that no promotion of the scrutinee itself could ever occur because "we can't ask for anything unless we already know we have it".

So why would the irrefutable pattern match need some special logic to prevent promotions of the scrutinee itself? Is it because of potentially throwing checks (var (P && _ as MyType) = promotableVariable;)? A quick test in DartPad indicates that promotableVariable does not get promoted by the as pattern, but that lack of promotion does not seem to be required by soundness.

eernstg avatar May 02 '25 08:05 eernstg

@eernstg

However, I'm not quite sure why we need this:

an irrefutable pattern match never promotes the type of the scrutinee itself.

If the irrefutable pattern match occurs as the behavior of a pattern variable declaration like var P = e; then the static type of e should be dynamic, or it should justify the viability of the match: If P is an object pattern built on a class C then the static type of e must be assignable to C; if P is a record pattern with specific required types for each component then the static type of e must be a record with the same shape and assignable component types, and so on. Technically, the requirement is that the static type of the initializing expression must be assignable to the required type of the pattern.

I'd expect this requirement to ensure that no promotion of the scrutinee itself could ever occur because "we can't ask for anything unless we already know we have it".

So why would the irrefutable pattern match need some special logic to prevent promotions of the scrutinee itself? Is it because of potentially throwing checks (var (P && _ as MyType) = promotableVariable;)? A quick test in DartPad indicates that promotableVariable does not get promoted by the as pattern, but that lack of promotion does not seem to be required by soundness.

I'm not going to try to defend that we "need" it; it's not necessary for soundness or anything. But for the record, we did explicitly decide it, back in https://github.com/dart-lang/language/issues/2857. The primary motivation was that we wanted to make ordinary assignments and variable declarations behave the same as the equivalent pattern assignment or pattern variable declaration (e.g., we wanted T x = value; to behave the same as var (T x) = value;); since ordinary assignments and variable decalrations don't promote their RHS, we decided that pattern assignments and pattern variable declarations shouldn't. If folks think that's confusing, and would rather make pattern assignments and pattern variable declarations consistent with refutable patterns (which do promote the scrutinee), that's certainly a change we could make.

stereotype441 avatar May 02 '25 22:05 stereotype441

Should implicit downcast promote?

If we have

dynamic d1 = 42, d2 = 42;
int i1 = d1 as int, i2 = d2;

we promote d1 because of the explicit as int, but we also perform an implicit as int downcast on d2. Should the implicit downcast promote too?

(Why not? The cast is there, and the author just uses a feature that allows it to be implicit.)

There are non-refutable declaration patterns which do type assertions too:

  Object? oq = ...;
  var (o!) = oq;
  var (i as int) = oq;

Do they promote? ... Nope, they do not.

void foo(bool b, Object? oq) {
  Object ov = 0;
  int iv = 0;
  Object? oq = null as Object?;
  if (b) {
    // Guard to contain promotions.
    var o = oq!;
    oq.st<E<Object>>(); // Promoted.
  }
  if (b) {
    var (o!) = oq;
    oq.st<E<Object?>>(); // Not promoted.
  }
  if (b) {
    (ov!, iv) = (oq, 42);
    oq.st<E<Object?>>(); // Not promoted.
  }
  if (b) {
    if (oq case var o! when oq.st<E<Object>>()) {
      oq.st<E<Object>>(); // Promoted
    } else {
      oq.st<E<Object>>(); // False branch too.
    }
  }

  if (b) {
    var i = oq as int;
    oq.st<E<int>>(); // Promoted
  }
  if (b) {
    var (i as int) = oq;
    oq.st<E<Object?>>(); // Not promoted
  }
  if (b) {
    (ov, iv as int) = (0, oq);
    oq.st<E<Object?>>(); // Not promoted
  }
  if (b) {
    if (oq case var i as int when oq.st<E<int>>()) {
      oq.st<E<int>>(); // promoted
    } else {
      oq.st<E<int>>(); // False branch too.
    }
  }
}

void main() {
  try {
    foo(false, null);
    foo(true, 42);
  } on Object {}
}

extension<T> on T {
  bool st<X extends E<T>>() => false;
}

typedef E<T> = T Function(T);

So even though we have flow that goes through an unconditional ! or as, we do not promote the variable that provided the matched value when it's a declaration or assignment pattern, only when it's a case pattern. Then we do promote, whether it matches or not.

lrhn avatar May 03 '25 06:05 lrhn

@lrhn

Should the implicit downcast promote too?

(Why not? The cast is there, and the author just uses a feature that allows it to be implicit.)

I definitely see your point. I also was also fairly swayed by past Lasse when he said this:

My thoughts on dynamic promotion: Irrefutable pattern contexts should not promote, refutable patterns should.

In a refutable/check context, you perform a type check, which makes the checked type a type of interest.

In a declaration/assignment context, the pattern does not imply any checks, no more than dynamic d= ...; int x = d; does. The implicit downcast from dynamic does not count as a check that introduces a type of interest, so no promotion. (And even if the type is already a type of interest for the variable, I still wouldn't promote the RHS of the assignment.)

I honestly don't know which version of Lasse I agree with more 😃

stereotype441 avatar May 06 '25 18:05 stereotype441

Past Lasse had the correct idea about assignment. An irrefutable pattern match is mostly like an assignment, and should behave the same.

Currently that means that implicit downcast doesn't count as a type check that introduces a type of interest. Maybe it should, but for both normal assignment and pattern assignment.

dynamic d = ...;
int x = d;
// or
var (int x) = d;

These should behave the same. I wouldn't mind if both counted as doing as int on the RHS of the assignment, which promotes d. Same for var A(_d: int x) = a; where a._i is a promotable field with type dynamic. This code will either have an int value or throw, which is what we otherwise use to promote.

(If we can remember that a variable has been upcast, so that a test on the upcast value counts as a test on the variable, then we could make var x = a._i as int; equivalent to int x = a._i as dynamic;, which is a rewrite used by web code to avoid doing the actual type check.)

The places where

an irrefutable pattern match never promotes the type of the scrutinee itself.

isn't correct is where it's not just an assignment: var (x as int) = a._i; and var (x!) = a._i; should both promote the scrutinee, y, of its a promotable variable, just as if the cast had been on the RHS. It's the same cast on the same value of the same variable.

lrhn avatar May 08 '25 05:05 lrhn

More generally, I think it would be beneficial if we define promotion in terms of logical operations, not syntax.

  • When you read a promotable variable, you remember that that value represents the variable.
    • If that variable is currently promoted, so is the value you read.
  • Some operations preserve the value and the link to the variable.
  • When you type-check a value which represents a variable, then
    • the checked type (plus/minus nullability) becomes a type of interest for the variable.
    • the variable is promoted on a true branch of the check result.
  • When you write a new value to a promotable variable, you may demote and/or promote to a type of interest.

(Plus all the usual things that prevent or cancel promotion.)

Then we should define which syntax performs a read, a type check and a write, and which preserve the link.

For local variables, reading is easy, it's a reference to the variable For a promotable private instance variable, we need to have a stable receiver. Then all of these read the value: r._i, r.._i, R(_i: _), r?._i, r?.._i. The last two only does so if r is not null, but on that branch, it reads _i. (There is not a lot you can do to promote in a selector chain, though. If we had a suffix/selector-cast, then r?.._i.as<int>.toRadixString(r._i - 1).logThis() could work.)

Operations that preserve the link includes parentheses, (r._i), and cascades, r._i..whatever, but also storing in a temporary record and taking the value out again, like (x._i, 42).$1. Also, going from matched expression of a switch or an if/case to the pattern. (We could choose to include assignment as preserving a binding. An x = y is defined to evaluate to the same value as y, just like y..selector does. It's confusing because if x is a promotable variable, then x = y is now also the value of x, which may be what the user would expect to be promoting, and we don't want to track that the value is linked to both ... right? So currently if ((x = y) is int) .... promotes neither x nor y.)

Operations which type check include expressions v as T, v! and v is T, and patterns T x, T(...), p?, p as T, p!, v?.s, v?..s. The ass and !s promote or throw immediately, so they promote in the continuation without any explicit branch. The rest test and promote on one of the the continuations. (And we could choose to include implicit downcast from dynamic if we wanted to, since it works exactly like an implicit as.) The T x, T(...) and p as T patterns introduce T (+/- nullability) as a types of interest. The p? and p! patterns and v?.s and v?..s null-aware member accesses don't need to add types of interest, because the current type of the variable should always be a type of interest already (and always +/- nullability along with it).

Type checking with a record type tests the individual fields, which can promote those if they are linked to a variable. (So (r._x, r._y) as (int, int) will promote both r._x and r._y to int.)

Assigning a linked value to a new variable breaks the link. That value is now linked to the new variable, so we only need to track connections from values to variables through expressions (and into switches). So no var pair = (x, y); if (pair is (int, int)) { x =+ y } indirect promotion through another variable, even if if ((x, y) case (int, int) _) x += y; works.

The reason for trying to define this in terms of logical operations, is that it makes it more likely that we remember to include new language features in the promotion. We don't have to look at them individually and say "does this look like it should promote". We say:

  • "does it read a promotable variable?",
  • "Does it preserve the value of an expression (in a way where it's meaningful to preserve a link to a variable)?"
    • including "is it a composite value which can meaningfully remember variable bindings of its components?")
  • "Does it perform a type check on a value?" (aka. "does it branch depending on the type of the value, or provide a boolean one can branch on?").

Then we can go back and look at all our operations and see which new syntax matches which kind of operation.

Not sure if dot-shorthands can use this, they're about statics, which is one thing we don't promote. But let's say that we did promote static final variables in the same library, which we should. Then Foo? defaultFoo = (._cache?.self ??= ._cache.initial); could work if Foo._cache is a final static variable of type FooCache? (some configurations use a cache, which can be cleared, but has a default value if read when cleared? Suspend your disbelief!), where FooCache has a Foo? self variable. The context type both the shorthands is Foo? or Foo, but the .self ??= ... is only evaluated if .cache is not null, so ._cache.initial would refer to the promoted value of Foo._cache and wouldn't need the ?. Looking at that as logical operations, .cache (implicit Foo.cache) is a read of a promotable variable, ?.self is a test promoting that variable to non-null FooCache in the selector continuation, ??= gives the RHS a context type of Foo? so ._cache is another read in a promoted state, so .initial does not need a ?.

Nothing revolutionary, just making .cache a promotable variable read, the rest is existing operations. As can be witnessed by this being OK:

class Foo {
  final FooCache? _cache;
  Foo(this._cache);
  
  Foo? get defaultValue => _cache?.value ??= _cache.initialValue;
}
class FooCache {
  Foo? value;
  Foo? initialValue;
}

lrhn avatar May 08 '25 07:05 lrhn

Is this only an issue around implicit downcasts? I can't think of how you could have a pattern variable declaration where promotion would actually do anything otherwise.

If so... my pitch would be to eliminate implicit downcasts. :)

munificent avatar May 12 '25 18:05 munificent

Also explicit casts: var (x as T) = y; and var (x!) = y;. Not particularly necessary by themselves, but can be used inside record destructing too.

lrhn avatar May 13 '25 22:05 lrhn

Based on the conversations above, I see several options for how we might move forward:

  1. Don't change the current status quo. Type tests implied by refutable patterns can cause the scrutinee or its fields to be promoted (assuming they're promotable). Type tests implied by irrefutable patterns never cause any promotions.
  2. Change the behavior of irrefutable patterns so that they can promote fields of the scrutinee (or fields of fields of the scrutinee, etc). But an irrefutable pattern still will never promote the scrutinee. Advantage: allows more promotion than option 1, but maintains behavioral consistency between ordinary variable declarations and pattern variable declarations, as well as behavioral consistency between ordinary variable assignments and pattern assignments.
  3. Change the behavior of irrefutable patterns so that they can promote the scrutinee or its fields. Advantage: allows even more promotion than option 2. Disadvantage: ordinary assignments (x = y) and pattern assignments ((x) = y) will no longer behave the same: if the RHS has type dynamic, the pattern assignment will promote; the ordinary assignment won't. Similar issue for ordinary variable declarations (int x = y;) and pattern variable declarations (var (int x) = y;).
  4. Same as option 3, but also change the behavior of ordinary assignments and ordinary variable declarations, so that if the RHS has type dynamic, promotion occurs. Advantage: allows even more promotion than option 3. Advantage: maintains behavioral consistency between ordinary variable declarations and pattern variable declarations, as well as behavioral consistency between ordinary variable assignments and pattern assignments. Disadvantage: there is still some inconsistency, because some implicit downcasts from dynamic promote, while others don't.
  5. Same as option 4, but make all implicit downcasts promote. Advantage: even more promotion than option 4. Advantage: very consistent.

Option 1 is a no-op.

I've prototyped options 2 and 3. They're both straightforward to implement and quite non-breaking in practice (they introduce zero test failures in google3). I would feel comfortable rolling either of these options into the sound_flow_analysis feature flag (which is being enabled for Dart 3.9), provided that we make the decision soon enough that I can get the changes landed before Beta 3.

Options 4 and 5 are significantly more work, but potentially leave us with a much more consistently language. If we went with one of those options it would have to be postponed until a later release.

Another possibility we could consider would be to do option 2 or 3 now, and postpone options 4 and 5 until a later release.

I'll add an agenda item to tomorrow's language meeting in case we want to discuss these options over VC.

stereotype441 avatar Jun 10 '25 20:06 stereotype441

We discussed this in today's language team meeting and decided to go with option 3: Change the behavior of irrefutable patterns so that they can promote the scrutinee or its fields.

stereotype441 avatar Jun 18 '25 15:06 stereotype441