Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve type inference for List (and maybe other generics). #980

Closed
ayporos opened this issue May 22, 2020 · 10 comments
Closed

Improve type inference for List (and maybe other generics). #980

ayporos opened this issue May 22, 2020 · 10 comments
Labels
request Requests to resolve a particular developer problem

Comments

@ayporos
Copy link

ayporos commented May 22, 2020

Type inference can be improved for Lists.

Let's take a look at the following code:

class A {
  A(this.name);
   String name;
}

class B extends A {
  B(String name): super(name); 
}

void main() {
  var list = generate();
  print(list.runtimeType); // List<B>
}

List<A> generate() { // method returns List<A>
  var list = [B("B 1"), B("B 2")]; // type inference kicks in -> list is List<B>
  list.add(B("B 3"));
  return list; 
  }

In this made up example you can see that actual type would be List<B>.
Trying to add A object to this list will lead to TypeError:..

As a proposals can it be that

  • compiler will take care of creating "correct" type of var list = [B("B 1"), B("B 2")]; (List<A> in this example same as function return type)
    OR
  • at least warning about type mismatch

Real life example of flutter code:

List<Widget> createFrom(List<MyDataItem> items) {
      items
      .map((item) => Expanded(...))
      .toList()
} 

Without changing map to map<Widget> one will not be able to add to this list later on anything except Expanded widget or its subtypes and it's actually hard to find out why (worse if code is not available to you)

@ayporos ayporos added the request Requests to resolve a particular developer problem label May 22, 2020
@eernstg
Copy link
Member

eernstg commented May 25, 2020

Dart class generics is a dynamically checked covariant mechanism, which means that List<B> is a subtype of List<A>, and the actual type of the argument passed to add of a list will be checked at run time. (In general, non-covariant usages of a type variable in a class give rise to dynamic checks.)

This is the same trade off between static and dynamic checks that Java and C# have for arrays (which may cause ArrayStoreException resp. ArrayTypeMismatchException at run time).

Over several years, Dart has changed in the direction of supporting purely static checking.

In the specific area of class generics, we are likely to add support for declaration site variance (#524) and use-site variance (#753) to the language, which would make it possible for you to express the example in a way which has no dynamic checks (that is: it is statically type safe).

class A {
  A(this.name);
  String name;
}

class B extends A {
  B(String name): super(name); 
}

void main() {
  var list = generate();
  list.add(A("Another A")); // Safe.
}

List<exactly A> generate() {
  return [B("B 1"), B("B 2")]; // Inference makes it a `List<A>` by context.
}

// The previous version of `generate` needs to be adjusted:
List<exactly A> oldGenerate() {
  var list = [B("B 1"), B("B 2")]; // type inference kicks in -> list is List<B>
  list.add(B("B 3"));
  return list; // Compile-time error: `List<B>` is not a subtype of `List<exactly A>`.
}

This shows that you can restrict a legacy type like List to be treated as invariant (using exactly), and it relies on use-site invariance. If we go with use-site variance (a more general model), the return type could be List<inout A>, but that makes no difference for this example.

With declaration-site variance the variance would be made part of the class itself, which is more convenient and concise. However, it is likely to be a far too radical breaking change to make a class like List invariant.

So the response to this issue is basically: Yes, Dart class generics do currently rely on dynamic checks for non-covariant usages of type variables. Support for variance which relies only on static checks is coming.

@ayporos
Copy link
Author

ayporos commented May 25, 2020

I'm not sure if I was clear enough. My problem is not with covariant generics but with a compiler not able to find out expected type.
E.g.

List<A> generate() {
   return [B("B 1")];
}

will return List<A>
But

List<A> generate() {
  var list = [B("B 1")];
  return list; 
}

will return List<B>!

One more flutter example:

class MyDataItem {
  MyDataItem(this.isText);
  bool isText;  
}

List<Widget> createFrom(List<MyDataItem> items) => items
      .map((item) => item.isText ? Text("Wow") : Banner(message: "Banner", location: BannerLocation.bottomEnd))
      .toList();
}

void test() {
  List<MyDataItem> items = List<MyDataItem>();
  items.add(MyDataItem(true));
  var widgets = createFrom(items);
}

Adding following code to test() will work:

  // no error:
  widgets.add(Container(child: Text('Hello, World!')));

But adding this one will lead to an error:

  // error:
  widgets.add(SizedBox(
      width: 200.0,
      height: 300.0,
      child: const Card(child: Text('Hello World!')),
  ));

Why? Because both Text and Banner are inherited from StatelessWidget -> createFrom returns List<StatelessWidget>. And it's possible to add Container to it (also StatelessWidget) but it's not possible to add SizedBox (inherited from RenderObjectWidget)!
Should app developer know about such things and keep them mind when developing? I'm doubt it.

What is worse that previously working code like this one:

List<Widget> createFrom(List<MyDataItem> items) => items
      .map((item) => item.isText ? Text("Wow") : 
            SizedBox(
                 width: 200.0,
                 height: 300.0,
                 child: const Card(child: Text('Hello World!')),)
      )
      .toList();
}

by a simple change (replacing SizedBox widget with a Container widget) can lead to a runtime error:

List<Widget> createFrom(List<MyDataItem> items) => items
      .map((item) => item.isText ? Text("Wow") : Container(child: Text('Hello, World!'))
      )
      .toList();
}

@eernstg
Copy link
Member

eernstg commented May 25, 2020

I'm not sure if I was clear enough. My problem is not with covariant
generics but with a compiler not able to find out expected type.
E.g.

List<A> generate() {
   return [B("B 1")];
}

will return List<A>
But

List<A> generate() {
  var list = [B("B 1")];
  return list; 
}

will return List<B>!

This is working as expected, but it may come as a surprise to you that inference in Dart gives priority to the context type (that is, the type required by the enclosing construct).

The first return statement has List<A> as the context type for the returned list, and it is possible for a List<A> to contain a B, so inference succeeds and makes it return <A>[B("B 1")];.

In the second example, you create a list with no context type (var doesn't convey any information about which kind of list you want), so you get bottom-up inference which makes it var list = <B>[B("B 1")]. Then you return list, which is allowed because List<B> is a subtype of List<A>. This is again so because class generics in Dart is covariant, with dynamic checks for non-covariant usages of type variables.

You cannot currently make return list; a compile-time error in this situation, but you will be able to do this if and when we get support for sound variance.

With createFrom, it looks like you need to create a List<Widget> (in order to avoid the unwanted covariance), such that subsequent additions of any kind of Widget will succeed.

List<Widget> createFrom(List<MyDataItem> items) => items
    .map<Widget>((item) => item.isText
        ? Text("Wow")
        : Banner(message: "Banner", location: BannerLocation.bottomEnd))
    .toList();

This will eliminate the subsumption where createFrom returns a List<StatelessWidget> (or something similar) when it is intended to return a List<Widget>.

However, in order to support this property by a static check we'd need something like use-site invariance. The return type could then be changed to List<exactly Widget>, which would statically enforce that the returned object is a List<Widget> and not a List<T> for some subtype T of Widget.

@ayporos
Copy link
Author

ayporos commented May 26, 2020

but it may come as a surprise to you

I'm not sure if I'm the only one who might be surprised to be honest.

Then you return list, which is allowed because List<B> is a subtype of List<A>.

I'm not saying it's not allowed. What I'm saying that compiler knows by function definition that it should return List and it's possible for compiler to enforce List<A> instead of List<B>
which would be safer option in my opinion.

Looks like because my example was really small it's quite easy to say "works as expected". But in reality it can be much harder to argue/debug such behavior.
As an example:

List<A> create() {
  if (Random().nextBool()) {
    var list = [B()];
    return list;
  } else {
    return [A()];
  }
}

Does anyone expect this function to return different types? But it does. I cannot say how hard it is for a compiler in this case to take into account surrounding context like "var list will be returned from this function so let's make it List<A> instead of List<B>". I think it's possible but correct me if I'm wrong.

With createFrom, it looks like you need to create a List

I know how to "fix" it as I mentioned in the original post. But do you think that everyone know/will be cautious enough to use the map<Widget>? Point is it's possible to write unsafe code without any warning that will work for now but might lead to runtime error in a future.

And to understand you better what's you saying is it's hard/impossible to improve type inference in this case or in your opinion everything works as expected?

P.S.:
I think one of lint rules will complain about List<A> list = <B>[B("B 1")] and ask you to replace it with var. Not sure if there is any auto-correction in Dart but anyway you can see to what it can lead if someone will just change it to a suggestion.

@ayporos
Copy link
Author

ayporos commented May 26, 2020

I'm not good at Kotlin but you will have an error

fun test(): MutableList<A> {
    var list = mutableListOf(B());
    return list;
}

Screen Shot 2020-05-26 at 11 50 08 AM

Which is nice in my opinion. You can always "opt-out" from type inference and fix it:

fun test(): MutableList<A> {
    var list = mutableListOf<A>(B());
    return list;
}

@eernstg
Copy link
Member

eernstg commented May 26, 2020

@ayporos wrote:

I think it's possible but correct me if I'm wrong.

Even if it might be possible, I don't think it is tractable or usable in practice to infer the type of a variable in a language with mutable state based on all its usages. It's been a firm rule for Dart type inference from the beginning that inference for a local variable can rely on the initializing expression, not all usages.

Another matter is that a local variable may get a different type in a limited scope based on promotion (for instance if (x is T) { /*x has type T here, under some conditions*/ }).

Your examples keep confirming that statically checked variance will be a useful addition to Dart. The Kotlin example shows the same thing, because Kotlin variance is statically checked (it has to be, because there is no reification of type arguments at run time, so a dynamic check cannot be implemented).

That, on the other hand, means that things like if (x is List<T>) {...} also cannot be implemented in Kotlin (there is an exception which can only be used with inline functions, cf. this article).

With statically checked variance in Dart, you can have the compile-time error as will as the dynamic test (using is), but we don't have the statically checked variance yet, so you'll unfortunately have to avoid covariance manually until then, in the situations where that is needed.

@ayporos
Copy link
Author

ayporos commented May 26, 2020

statically checked variance

Sorry but I'm not following. My complain is not about variance but about type inference and type inference is static (is it not?). So basically why is it not possible to have a warning/error here:

List<A> generate() {
  var list = [B("B 1")];
  return list;  // warning or error here -> list was inferred to be List<B> but return statement expecting List<A>
}

same way Kotlin does it? Just so developer will be aware of possible problems?

@eernstg
Copy link
Member

eernstg commented May 26, 2020

My complain is not about variance but about type inference
and type inference is static

Here's the part where I explained why type inference for a local variable in Dart will not take all usages in the scope of the variable into account, it will only use the initializing expression:

Even if it might be possible, I don't think it is tractable or usable in practice
to infer the type of a variable in a language with mutable state based on all
its usages. It's been a firm rule for Dart type inference from the beginning
that inference for a local variable can rely on the initializing expression, not
all usages.

The particular change to inference that might make a difference (and do something that matches your request), will not happen in Dart. It doesn't fit into a language with mutable state and subsumption, because it immediately gets intractable if you make an attempt to trace where each object goes, and then solve the type inference with respect to all the variables that may be involved.

However, if you want static type safety (rather than a failing type check at run time), you need some form of statically checked variance. Kotlin variance is statically checked (and it's basically the same thing as Java wildcards, and they have the same property).

As long as you have covariance for class type variables the return list where list has type List<B> will be accepted (and all usages of that list where the type variable is in a covariant position will be safe, and statically known to be safe, but non-covariant signatures are known to be statically unsafe, and they will be checked dynamically).

@ayporos
Copy link
Author

ayporos commented May 26, 2020

As long as you have covariance for class type variables the return list where list has type List<B> will be accepted

Yes, this one I understand and I'm not argue with that.

because it immediately gets intractable if you make an attempt to trace where each object goes

I got your point.
What I was trying to say that I'm ok with explicit things like:

List<A> generate() {
  List<B> list = [B("B 1")];
  return list;
}

or

map<StatelessWidget>(...).toList()

And at the same time I find it hard to understand following code because of its implicit nature and no warnings from the compiler:

List<Widget> createFrom(List<MyDataItem> items) => items
      .map((item) => item.isText ? Text("Wow") : Container(child: Text('Hello, World!')))
      .toList();
}

But from

It's been a firm rule for Dart type inference from the beginning
that inference for a local variable can rely on the initializing expression, not
all usages.

looks like it is how it is.
Thank you for you explanation and patience! Please fill free to close this issue.

@eernstg
Copy link
Member

eernstg commented May 27, 2020

Thanks! An example like createFrom is a very good illustration of a situation where the dynamically checked covariance is dangerous, so this adds further support for the introduction of statically safe variance.

@eernstg eernstg closed this as completed May 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request Requests to resolve a particular developer problem
Projects
None yet
Development

No branches or pull requests

2 participants