Infer generics when doing pattern matching

[rrousselGit]

Hello!

It appears that currently, when we write:

class Generic<T> {
  final T value;
}

...
switch (Generic<int>()) {
  case Generic(:final value?):
}

then value is inferred as "dynamic". But in this context, T is guaranteed to be at least int

Would it be possible to consider case Generic(<...>) as case Generic<int>(<...>)?

[lrhn]

My immediate thought is that the pattern should infer <int> as type argument to the Generic object pattern type, bare on the matched value type.
Then the value variable should be inferred as NonNull(int), aka int.

Either a bug, or I'm forgetting something. Have to do a thorough reread of the pattern inference rules to be sure.

(A quick reread does look like it says what I remembered it as saying: For an object pattern, do downwards inference on the type using the matched value type as context type, to infer type arguments if applicable. Then look up the member getters on that type to give the matched value type for the property patterns. So probably an implementation bug that should be moved to the SDK repo.)

[natebosch]

I cannot reproduce this. In both the analyzer and the VM the static type of value is correct.

class C<T> {
  final T value;
  C(this.value);
}

Type typeOf<T>(T value) => T;

void main() {
  var x = switch (C<int>(1)) {
    C(:final value) => typeOf(value),
  };
  print(x);
}

Do you have a more complete reproduction that demonstrates this?

[rrousselGit]

Ah indeed this happens in some scenarios.

The case where I've faced this is with AysncValue from Riverpod.

Here's an extracted version:

void main() {
  switch (AsyncData<int?>(42)) {
    case AsyncValue(:final value?):
      print(value);
  }
}

abstract class AsyncValue<T> {
  T? get value;
}

class AsyncData<T> extends AsyncValue<T> {
  AsyncData(this.value);

  @override
  final T value;
}

[lrhn]

That does give value a static type of dynamic. And it does look like a bug.

The static type of AsyncData<int?>(42) is AsyncData<int?>, which implements AsyncValue<int?>.

The case AsyncValue(:final value?) should, I hope, but cannot guarantee, find that its matched value type
implements AsyncValue<int?>, and infer int? as parameter type.
Then .value has static type int? (int?? even, but that should be the same thing), and :final value? matched against that should bind value to int.

The most likely cause of the result is that the pattern infers AsyncValue<dynamic>(:final value?) instead.

All the specification says is "perform downwards inference" trying to match AsyncValue(...) to AsyncData<int?>.
If we had AsyncValue<T> as context type for an AsyncData<int?>, we'd infer T = int. (But that's upwards inference, not downwards?)

So, either a bug, or the specification is unclear.

[victorem1]

Just a bump that I've also been affected by this.

[lrhn]

Either bug in specification or implementation. We'll have to decide.

[johnniwinther]

cc @stereotype441

[eernstg]

The matching operation P <# Q [L] -> C which is used to generate constraints has a built-in direction: It will compute a set of constraints C on type variables in L such that P1 <: Q1 is satisfied, where P1 and Q1 are obtained by substituting the solution to C into P respectively Q.

If we've trying to compute the constraints on T such that AsyncValue<T> <: AsyncData<int?> then we will indeed find that there are no constraints that will produce this outcome, and hence the inferred value of T is unconstrained, which may well cause it to be bound to dynamic.

It's a bit like this (which won't be completed anyway because it's a compile-time error):

void f<X>(List<X> list) {}

void main() {
  Iterable<int> xs = [];
  f(xs); // `f<int>` yields a compile-time error.
}

I think the crucial point here is that the matched value type is of the form B<T1 .. Tk>, and we're considering an object pattern whose class is a superclass A of B. The matching machinery is built to handle the case where the object pattern can be a subtype of the matched type (and we'll infer actual type arguments for the object pattern to make that happen), so it basically can't handle a superclass.

So perhaps the algorithm where actual type arguments are computed for an object pattern should have an extra step: If the object pattern has a class A that isn't the class of the matched value type B<T1 .. Tk>, but A occurs in the superinterface graph of B<T1 .. Tk> (in the example: AsyncValue occurs in the superinterface graph of AsyncData<int?>) then the inference step should consider the matched value type to be the superinterface of B<T1 .. Tk> which is a generic instance of A (in the example: the matched value type should be considered to be AsyncValue<int?>).

Alternatively, we could just warn the developer that the desired subtype relationship can't happen, and hence they don't actually want to write case AsyncValue(:final value?), it should be case AsyncType(:final value?). It could be a lint, object_pattern_is_upcast, or something like that.

[haf]

Another example, but not for subclasses of the current instance, but for subclasses of the generic argument:

@immutable
sealed class EntitySelectorState<T extends BaseEntity> {
  const EntitySelectorState({
    this.lastDocumentId,
    this.currentQuery = '',
    this.isLoadingMore = false,
  });

  final String? lastDocumentId;
  final String currentQuery;
  final bool isLoadingMore;

  // A homomorphism on the discriminated union of the selector state
  K map<K>(
    K Function(InitialEntitySelectorState<T>) onInitial,
    K Function(LoadingEntitySelectorState<T>) onLoading,
    K Function(SuccessEntitySelectorState<T>) onSuccess,
    K Function(FailureEntitySelectorState<T>) onFailure,
  ) {
    return switch (this) {
      InitialEntitySelectorState<T> state => onInitial(state),
      LoadingEntitySelectorState<T> state => onLoading(state),
      SuccessEntitySelectorState<T> state => onSuccess(state),
      FailureEntitySelectorState<T> state => onFailure(state),
      // I want to remove these:
      InitialEntitySelectorState<BaseEntity>() => throw AssertionError('Unexpected (initial) state: $this'),
      LoadingEntitySelectorState<BaseEntity>() => throw AssertionError('Unexpected (loading) state: $this'),
      SuccessEntitySelectorState<BaseEntity>() => throw AssertionError('Unexpected (success) state: $this'),
      FailureEntitySelectorState<BaseEntity>() => throw AssertionError('Unexpected (failure) state: $this'),
    };
  }
}

Here, we know T as constant throughout the class / method declaration.

[eernstg]

It seems likely that you would want to restrict the type argument of BaseEntity in the bound of T in the selector state classes, but otherwise it seems to work at this time.

// Glue code.

const immutable = 1;

class BaseEntity<T> {}

abstract class InitialEntitySelectorState<T extends BaseEntity>
    implements EntitySelectorState<T> {}

abstract class LoadingEntitySelectorState<T extends BaseEntity>
    implements EntitySelectorState<T> {}

abstract class SuccessEntitySelectorState<T extends BaseEntity>
    implements EntitySelectorState<T> {}

abstract class FailureEntitySelectorState<T extends BaseEntity>
    implements EntitySelectorState<T> {}

// Original example.

@immutable
sealed class EntitySelectorState<T extends BaseEntity> {
  const EntitySelectorState({
    this.lastDocumentId,
    this.currentQuery = '',
    this.isLoadingMore = false,
  });

  final String? lastDocumentId;
  final String currentQuery;
  final bool isLoadingMore;

  // A homomorphism on the discriminated union of the selector state
  K map<K>(
    K Function(InitialEntitySelectorState<T>) onInitial,
    K Function(LoadingEntitySelectorState<T>) onLoading,
    K Function(SuccessEntitySelectorState<T>) onSuccess,
    K Function(FailureEntitySelectorState<T>) onFailure,
  ) {
    return switch (this) {
      InitialEntitySelectorState<T> state => onInitial(state),
      LoadingEntitySelectorState<T> state => onLoading(state),
      SuccessEntitySelectorState<T> state => onSuccess(state),
      FailureEntitySelectorState<T> state => onFailure(state),
    };
  }
}

[haf]

Thank you; I discovered that the constraint caused a default inferral in the base classes;

class SelectorState<T extends BaseEntity> ...
class LoadingEntitySelectorState<T extends BaseEntity> extends SelectorState {}

which implicitly made it class LESS<T extends BaseEntity> extends SelectorState<BaseEntity> instead of class LESS<T extends BaseEntity> extends SelectorState<T>, which caused the problem in the switch statement.

In TypeScript, this would not have happened (it would error on the subclass declarations), because it would have required this declaration: T extends BaseEntity = BaseEntity in the SelectorState declaration for a default to be used.

[eernstg]

In TypeScript, this would not have happened (it would error on the subclass declarations), because it would have required this declaration: T extends BaseEntity = BaseEntity in the SelectorState declaration for a default to be used.

Yes, we discussed having the notion of default values of type parameters, but instead we're using a process which is called instantiation to bound. Support for defaults could be added in the future (see dart-lang/language#283).

In the case (the rather common case, I think) where explicitly provided type arguments are required as a matter of style, you can enable strict-raw-types in your analysis_options.yaml file.

I'm afraid that strict-raw-types is a bit less strict than I would have preferred. I think it's happily using instantiation to bound as long as it doesn't yield any actual type arguments whose value is dynamic. However, strict-raw-types will catch this particular case and a lot of others, because any type parameter with no bound will be bound to dynamic by instantiation to bound.