language icon indicating copy to clipboard operation
language copied to clipboard

Less than ideal type inference for type parameters in some cases

Open osa1 opened this issue 3 months ago • 6 comments

(Please feel free to update the title with a more descriptive one.)

Repro from @mraleph:

class A<T> {}

class B<T> extends A<T> {}

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

void main() {
  B<String>? b;
  print(typeOf(b ?? A()));
}

Here ideally we want to see String, but it prints dynamic instead.

So in b ?? A(), the type parameter of A is inferred as dynamic instead of String even though type of b is B<String>.

In practice, this causes a lot of headache in a change in the protobuf library. Recently we made a change to return more precise types in generated message classes. Instead of List<T>, we now return PbList<T>. PbList<T> extends ListBase<T> which implements List<T>.

This change caused a lot of breakage in user code, because a lot of types became dynamic after this change.

The fix is not always trivial: we have to add type annotations, but sometimes the types are not even in scope, and sometimes we can't even import the types because they're not exported in a direct dependency.

The fix is also triggers the omit_local_variable_types lint, which shouldn't be a lint as it changes semantics (without type annotations, the element type becomes dynamic).

The changes needed in user code can be seen in cl/809933607. There are two kinds of code that breaks with this change:

  • myMessage?.myList ?? []

  • var myList = myMessage.myList; // PbList<T>
    ...
    if (...) {
      myList = [...]; // List<T>
    }
    

osa1 avatar Sep 23 '25 08:09 osa1

There is ongoing work to enforce the use of Object? rather than dynamic during type inference in the situation where a type variable is unconstrained. This has been specified for a while (via the section grounded constraint solution for a type variable and greatest closure), but it is only now being consistently implemented. So that's the reason why A() gets inferred as A<dynamic>() rather than A<Object?>() when there are no constraints on the type variable. @chloestefantsova, do you have further comments on that?

The reason why there are no constraints on the type variable of A() in the expression b ?? A() in the empty context is that there is no T such that A<T> is a subtype of B<String>, which is the static type of b. This means that the situation doesn't fit the basic mechanism in the type inference constraint generation algorithm, which is based on P <# Q [L] -> C (that is, "P matches Q with respect to the set of type variables L if the set of constraints C is satisfied"). This is all about making P a subtype of Q, and if that definitely isn't going to be true then we can't generate any constraints.

So if the original example in cl/809933607 had a similar expression where the type of b used to be List<S> and it is now PbList<S>, and A() is [], then this will give rise to the same problem: There is no type T such that List<T> <: PbList<S>, and hence [] is inferred as <dynamic>[].

Considering that this creates substantial breakage, is it really important to have the more specific type PbList? Could the more precise type be provided via a new API, such that existing code that uses the current API won't break?

eernstg avatar Sep 23 '25 09:09 eernstg

Considering that this creates substantial breakage, is it really important to have the more specific type PbList? Could the more precise type be provided via a new API, such that existing code that uses the current API won't break?

I think a new API is not an option for protobuf, because once added nothing can be removed in protobuf, because of large number of users.

The point here is that if I have a value (it can be a field, or a function return value) and change its type to a subtype of the previous type, I get dynamic values. That seems counter intuitive. It also makes this type of change breaking.

This change has already been shipped in the open source, but I've been unable to sync it to internal so far because of the large breakage.

Unless we say "any type change should be considered breaking" when versioning, this kind of thing can also slip through, because it's difficult to consider all kinds of expressions using a value with values of different types (supertypes and subtypes) to detect this kind of thing. In protobuf apparently we didn't have any uses like myMessage?.myList ?? [] so we didn't realize that these expressions break with the change. I can have 100% test coverage for my library and still not check this kind of thing.

I'm not an expert in Dart type checking and I don't quite follow the context stuff above, if this can't be improved in Dart then we should close this.

In my case at least, making the type Object? instead of dynamic will probably make little difference, because dynamic is already disallowed in many of the use sites. So I'll get a compile-time error in both cases. But in general avoiding dynamic is a good idea so it should be preferable to have Object? here.

osa1 avatar Sep 23 '25 09:09 osa1

Thinking about this again, I don't think we'd have to revolutionize the type inference algorithm, it should be enough to adjust the computation of the context type for the right hand operand of ?? when the expression as a whole occurs in the empty context: https://github.com/dart-lang/language/issues/4516.

eernstg avatar Sep 23 '25 10:09 eernstg

There is ongoing work to enforce the use of Object? rather than dynamic during type inference in the situation where a type variable is unconstrained. This has been specified for a while (via the section grounded constraint solution for a type variable and greatest closure), but it is only now being consistently implemented. So that's the reason why A() gets inferred as A<dynamic>() rather than A<Object?>() when there are no constraints on the type variable. @chloestefantsova, do you have further comments on that?

I don't have much, except that inferring Object? instead of dynamic in similar cases is blocked on https://github.com/dart-lang/language/issues/4466. But, like mentioned in the discussion here, inferring Object? won't resolve the issue in question.

chloestefantsova avatar Sep 23 '25 10:09 chloestefantsova

Thinking about this again, I don't think we'd have to revolutionize the type inference algorithm, it should be enough to adjust the computation of the context type for the right hand operand of ?? when the expression as a whole occurs in the empty context: dart-lang/language#4516.

The update to the inference rules for ?? looks promising. However, it won't resolve all of the mentioned breakage. @osa1 also mentioned the following breakage pattern that, I believe, is very hard to address in the current design of the type inference in Dart:

  • Initializing a variable and then updating it later:
    var myList = myMessage.myList; // PbList<T>
    ...
    if (...) {
      myList = [...]; // List<T>
    }
    

chloestefantsova avatar Sep 23 '25 10:09 chloestefantsova

It's working as designed, so it's not an SDK issue. Moving to language repo as a request to improve the inference.

(I'm beginning to think we should always use the non-nullable type of the first operand, possibly after coercion to an actual context type, as context type for the second operand. After all, the value if the first operand can become a value of the outer context, if not null, so that type is a more precise sign of what kind of value is expected.)

lrhn avatar Sep 24 '25 10:09 lrhn