Strange type error with generic getters

[Hixie]

I don't really understand why this fails:

typedef Foo<T> = void Function(T value);

class _Property<T> {
  _Property(this.foo);
  Foo<T> foo;
}

void test<T>(Foo<T> foo) {
  _Property value; // or _Property<dynamic>, or _Property<Object>; works with _Property<T>
  value = _Property<T>(foo);
  value.foo; // this throws a type error shown below
}

void main() {
  test((int value) { });
}

->

Unhandled exception:
type '(int) => Null' is not a subtype of type '(dynamic) => void'
#0      test (file:///home/ianh/dev/scratch/test.dart:11:9)
#1      main (file:///home/ianh/dev/scratch/test.dart:15:3)
#2      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:301:19)
#3      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:168:12)

[leafpetersen]

This is a sharp edge of the interaction between covariant generics (unsound in general) and contra-variant type parameters. See here for more discussion, and the dart language repo issues on sound variance for more examples. TL;DR you're trying to read a field which contains an int -> Null at the type dynamic -> void which would let you pass anything where an int was expected. For soundness, we have to prevent this in some way, and the way we do it is by doing a type test on the read and throwing if the callback is not prepared to accept dynamic.

[Hixie]

I don't understand where I am telling the compiler that I want it to be "the type `dynamic -> void". I don't care what the type is. I know the type is correct or it wouldn't have been able to get through the compiler. I'm not assigning the value to anything.

[leafpetersen]

Property here is not a properly covariant generic. It is in general unsound to allow a Property<int> to be assigned to a Property<dynamic>. However, Dart still has covariant generics. This means that we must use runtime checks to recover soundness. This is the basic premise of the Dart 2 type system (it's a kind of sound gradual type system when it comes to generics, if you prefer that terminology).

In this specific case, value has type Property<dynamic>. Therefore, the static type of value.foo is dynamic -> void. This is not a correct static type for the thing that you are actually reading out of it. The actual thing you are reading out is an int -> void, and might do int things to its parameter. Therefore, when you do a read, we check at runtime that the thing that you are reading has a valid subtype of what you claimed was the static type (this check need only be done when there is a contravariant type parameter in the type of the thing being read). It's true that in your example you're not doing anything with the result of the read, and so it would be ok for us to let it happen. But in general, here are various ways it could go wrong if we did not do the checks:

typedef Foo<T> = void Function(T value);

class _Property<T> {
  _Property(this.foo);
  Foo<T> foo;
}

void test<T>(Foo<T> foo) {
  _Property value; // or _Property<dynamic>, or _Property<Object>; works with _Property<T>
  value = _Property<T>(foo);
  value.foo("hello"); // This is statically fine, and will end up adding 1 to "hello".
  var x = value.foo;
  x("hello"); // likewise 
  value = _Property<String>((String s) {});
  value.foo = x;  // We now have let an int -> void where a String -> void was expected
}

void main() {
  test((int value) { print(value + 1); });
}

So in general, letting reads of this kind happen unchecked breaks the type system. You're back in unsound land. There are other things you can try to do. Some gradual type systems would wrap the callback to check its argument dynamically - we don't do that for various reasons. We've considered changing the type during the read to give you something of type Never -> void which you can only call dynamically. This would let your example (where you don't do anything with the result) or other related examples where you only compare the result to null work, at the cost of requiring you to do something like (value.foo as dynamic)(3) to actually call it.

As I say, an unpleasant sharp edge to the compromises we made to keep covariant generics while making the type system sound.

[vsmenon]

Is there anything actionable here without language variance support (e.g., dart-lang/language#524 )?

Could we generate a better runtime error?

[Hixie]

I don't really understand why we need to check the type of foo here. The type has no follow-on effects. The value isn't invoked, dereferenced, assigned to anything, returned, passed, anything. We call the getter and then throw the returned value away.

[vsmenon]

In any realistic example, it will get used. @leafpetersen mentioned other options to delay the checking, but it will have some other (probably perf) cost.

E.g., in this part of @leafpetersen 's variant above, it needs to fail somewhere since the function x only accepts int:

  var x = value.foo;
  x("hello");

The old Dart 1 checked mode would do this check on every parameter inside the function x - that's a fairly expensive option in terms of perf and code size.

I think the nicer answer here is to statically prevent folks from mixing _Property and _Property<T> when a class has a field type where T is contravariant.

[leafpetersen]

In any realistic example, it will get used.

Right. We could definitely special case the exact situation where a field is read and then immediately discarded, but I think it essentially provides zero value. More useful would be to special case a few other situations: e.g.

• reading it and immediately comparing it to null
• using it as the RHS of an equality check
• immediately casting it to dynamic

Not sure those provide enough value, but could be worth considering. All other options we consider make one of two tradeoffs. Either they have a large non-local perf cost, or a local perf cost + odd identity semantics, or they result in you getting a value of an "unexpected" static type. The semantics that we have chosen has reasonable perf behavior, the identity semantics that you expect (reading a field doesn't the pointer identity of the object being read), gives you a value of the static type that you expect, and in the common case just works. Unfortunately, it does have the property that in the less common case it gives the surprising behavior you see.

[Hixie]

The problem is that the check happens before it's assigned, so I can't even do something like:

  dynamic x = value.foo as dynamic;

Instead I have to do:

  dynamic x = (value as dynamic).foo;

It makes zero sense to me as a developer that the first does not work.

[leafpetersen]

It makes zero sense to me as a developer that the first does not work.

Yes, this is a fair point. Enumerating a set of places where we could allow the read check to be omitted would be tedious and a bit brittle (e.g. dynamic x = value.foo would work, but var x = value.foo would have to be rejected), but it would allow some more reasonable looking patterns to work. I'll file an issue in the language repo to discuss.

[vsmenon]

@Hixie - the fundamental typing issue here is that we have a situation where the runtime type of value.foo is not a subtype of the static type of value.foo. That's a soundness violation and contagious - i.e., it can lead to cascading soundness failures. The simplest solution is to prevent value.foo from completing when soundness would be violated - the current behavior. That nips the problem in the bud. @leafpetersen is suggesting that we could allow the unsoundness but limit the contagiousness instead. I'm personally a bit wary of that direction.

For a developer, isn't the right outcome here getting them to change _Property value to _Property<T> value? I think any form of as dynamic feels like (and is) a hackaround.

[Hixie]

The code where I ran into this was where the _Property objects were being stored in a Map (mixed with properties of other types). I "fixed" it by just having a method on _Property to call foo.

The problem is that the failure mode is terrible. "type '(int) => Null' is not a subtype of type '(dynamic) => void'"? So what? I didn't ask for a type '(dynamic) => void'. I didn't ask for a type at all. I would be happy to assign it to a variable of type dynamic. Why doesn't the compiler complain, if it can fail at runtime? The current failure mode just makes the developer annoyed. At the absolute minimum we should have a better error message, but IMHO we should figure out a way to (without special casing certain patterns) either make this work or have the compiler catch it or something more ergonomic.

[johnniwinther]

@vsmenon @leafpetersen Can we direct this to an open language issue?

(Marking this issue as area-front-end seems a bit weird since I think we are doing what is spec'ed)

[eernstg]

The language issue dart-lang/language#297 is concerned with these caller-side checks, and it proposes that we give the relevant expressions a more general type (such that the static type is sound and there's no need for a check, no matter whether the value is used or discarded).

[leafpetersen]

I filed an issue around the meta problem with links to the appropriate proposals. Let's take further discussion there.