Downwards and upwards inference with object patterns

@nhannah commented on issue #3964. I'm going to move it to a separate issue here because I believe it's unrelated to the original topic of that issue. Their comment is:

I am interested if there would be a possibility to improve inference around this type alias object destructuring:

class Student {
  final String name;
  final int age;

  Student(this.name, this.age);
}

class StudentT<T> {
  final String name;
  final int age;
  final T something;

  StudentT(this.name, this.age, this.something);
}

StudentT<T> getStudent<T>(Student student, T Function(Student) selector) =>
    StudentT<T>(student.name, student.age, selector(student));

void main() {
  final StudentT(:age, :name, :something) = getStudent(Student('Ethiel', 12), (s) => s.age > 5);

  print('Student $name is $age $something years old');
}

In this example something is typed as dynamic as the generic type is lost.

void main() {
  final a = getStudent(Student('Ethiel', 12), (s) => s.age > 5);

  print('Student $name is $age $something years old');
}

a here is properly inferred as StudentT bool.

void main() {
  final StudentT<bool>(:age, :name, :something)  = getStudent(Student('Ethiel', 12), (s) => s.age > 5);

  print('Student $name is $age $something years old');
}

In this example something is properly typed as bool as it's explicitly set as the generic on StudentT.

void main() {
  final StudentT(:age, :name, :something)  = getStudent<bool>(Student('Ethiel', 12), (s) => s.age > 5);

  print('Student $name is $age $something years old');
}

This is where I get a slightly confused, getStudent can properly infer T as bool, but can't pass that inference to the destructuring. But if getStudent does not infer it's type and has it set explicitly this all works fine.

Swap to records and everything works as expected with no explicit types assigned but partial destructuring goes away:

typedef Student = ({
  String name,
  int age,
});

typedef StudentT<T> = ({
  String name,
  int age,
  T something,
});

StudentT<T> getStudent<T>(Student student, T Function(Student) selector) =>
    (name: student.name, age: student.age, something: selector(student));

void main() {
  final (:age, :name, :something) = getStudent((name: 'Ethiel', age: 12), (s) => s.age > 5);

  print('Student $name is $age $something years old');
}

I agree the behavior isn't ideal here. It's unfortunate, but I think I understand what's going on. Here's a simpler repro:

class Box<T> {
  final T value;
  String get type => runtimeType.toString();
  Box(this.value);
}

void main() {
  final Box(:type) = Box(123);
  print(type);
}

This prints Box<dynamic>.

Type inference works in three phases:

1. First we calculate a context type schema for the pattern (here `Box(:type)`).

The relevant part of the proposal is:

Object: The type the object name resolves to. This lets inference fill in type arguments in the value based on the object's type arguments, as in:
var Foo<num>() = Foo();
//                  ^-- Infer Foo<num>.
If the type the object name resolves to is generic, and no type arguments are specified, then instantiate to bounds is used to fill in provisional type arguments for the purpose of determining the context type schema. Note that during the type checking phase, these provisional type arguments will be replaced with the result of applying downwards inference. See "Type checking and pattern required type" below.

Since Box doesn't have an explicit type argument, instantiate-to-bounds gives a provisional context type schema of Box<dynamic>.

2. Then we use that to calculate a static type for the matched value (here `Box(123)`).

We take the Box(123) expression and run type inference on it with a context type of Box<dynamic>. Even though the context type schema Box<dynamic> was only "provisionally" filled in, it is a valid type and thus downwards inference stuffs that dynamic into the constructor call yielding Box<dynamic>(123). That takes precedence over the upwards inference we would get from 123.

Then the resulting static type of Box<dynamic>(123) is Box<dynamic>.

3. Then we take the result of that matched value type to fill in a complete static type for the pattern.

The relevant part of the proposal is:

Object:

Resolve the object name to a type X. It is a compile-time error if the name does not refer to a type. Apply downwards inference with context type M to infer type arguments for X, if needed. If any type arguments are left unconstrained, do instantiate to bounds (using the partial solution from downwards inference) to fill in their values.

So now we apply the matched value's type Box<dynamic> to the Box(:type) pattern and use it to fill in a type argument for the pattern yielding Box<dynamic>(:type).

I think that's what's going on.

What might work better would be that if instantiate-to-bounds doesn't have an actual bound to fill in for a type argument, it uses _ (the unknown type schema) instead of dynamic. Then when we apply Box<_> as the type schema to the initializer, the type argument doesn't get filled in, so inference instead infers it upwards from the 123.

But instantiate-to-bounds is way outside of my area of expertise, so I'm not sure if that would cause even worse problems elsewhere. @eernstg, @leafpetersen, or @stereotype441 would know better.

Record patterns don't have this problem because they don't do instantiate-to-bounds (since records never have bounds) when calculating a type schema:

Record: A record type schema with positional and named fields corresponding to the type schemas of the corresponding field subpatterns.

So with a record pattern, you will get a type schema like (_, _) if the field types aren't known, instead of (dynamic, dynamic).

Since Box doesn't have an explicit type argument, instantiate-to-bounds gives a provisional context type schema of Box<dynamic>.

I agree that that is the problem. It probably should instantiate with _ to avoid locking down a type during downwards inference.

The current pattern behavior matches the current behavior of non-pattern declarations: List l = [1]; will also instantiate to bounds and get <dynamic> everywhere. That's always been a horrible experience and foot-gun. We should fix that too.

It probably should instantiate with _ to avoid locking down a type during downwards inference.

Agreed! The only downside is the potential breakage (which is likely to be "good" breakage, by the way, because it is likely to cause some types to be more tight, e.g., replacing dynamic by a more specific type in a type argument).

dart-lang / language