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

Reading a closure variable that has a generic type parameter results in a runtime error #49419

Closed
JonathanPeterCole opened this issue Jul 7, 2022 · 3 comments
Labels
closed-as-intended Closed as the reported issue is expected behavior

Comments

@JonathanPeterCole
Copy link

I'm not sure if this is a bug or a known caveat of generic types, but I've noticed the following on Dart 2.17.5:

void main() {
  final TestClass<Object> testInstance = TestClass<double>(
    value: 123.45,
    closureA: () => 123.45,
    closureB: (double arg) {},
  );
  
  print(testInstance.value); // 123.45
  print(testInstance.method); // Closure: (Object?) => void from Function 'method':.
  print(testInstance.closureA); // Closure: () => double
  print(testInstance.closureB); // Throws "type '(double) => void' is not a subtype of type '(dynamic) => void'"
}

class TestClass<T> {
  TestClass({required this.value, required this.closureA, required this.closureB});
  
  final T value;
  final T Function() closureA;
  final void Function(T arg) closureB;
  
  void method(T arg) {}
}

Given the results of the other print calls, I would expect print(testInstance.closureB) to print either:

  • Closure: (double) => void
  • Or Closure: (Object?) => void

Due to this issue, code like the following won't work:

void main() {
  final TestClass<Object> testInstance = TestClass<double>((double? arg) {});
  if (testInstance.callback == null) { // Throws exception
    // ...
  }
}

class TestClass<T> {
  TestClass(this.callback);
  
  final void Function(T arg)? callback;
}
@JonathanPeterCole JonathanPeterCole changed the title Reading a closure variable with a generic type parameter results in a runtime error Reading a closure variable that has a generic type parameter results in a runtime error Jul 7, 2022
@lrhn
Copy link
Member

lrhn commented Jul 7, 2022

That looks like it's working as intended.

Just as we have extra runtime checks on methods like add(E) on List, because casting a List<int> to List<Object> makes it look like you can call .add("not an int") on it, we also have extra checks when reading properties where the type variable occurs contravariantly.

class C<T> {
  void Function(T) get f => (T x) {};
}
void main() {
 C<Object> c = C<int>();
 c.f; // Has static type `void Function(Object)` and actual value `void Function(int)`. That's unsound, so we throw.
}

@lrhn lrhn added the closed-as-intended Closed as the reported issue is expected behavior label Jul 7, 2022
@lrhn lrhn closed this as completed Jul 7, 2022
@JonathanPeterCole
Copy link
Author

JonathanPeterCole commented Jul 7, 2022

Ah that makes sense.

It would be useful if it matched the behaviour of methods, with the runtime check at the time the method is called, allowing for cases like this:

if (someClass.callback == null) // No need to type check the parameters here as it's not called
  // ...

But this does feel like a pretty rare case, and as a workaround bool get hasCallback => callback != null on the class does the job.

@eernstg
Copy link
Member

eernstg commented Jul 8, 2022

A while back, I created a proposal about the static type of "instance members whose type is contravariant". Such members are getters or instance variables whose type contains a type variable of the class in a non-covariant position, or methods including operators whose return type have that property.

The idea is that the static type is approximated soundly (from above). Some expressions will then have a more general type, and we may need to perform a downcast before we can use them. However, just throwing when the member is evaluated is is quite inconvenient to handle (basically, we'd need to try receiver.method(...) and if it throws we can do (receiver as dynamic).method(...)), so I think the more general (but sound) static type is preferable. For example:

class A<X> {
  void Function(X) m() => (x) {};
}

void main() {
  A<Object> a = A<int>();

  // Current behavior.
  a.m(); // Throws. Could use try/catch, but what would we do in `catch`?
  (a as dynamic).m(); // This doesn't throw, but it introduces dynamic types.

  // With the proposal where members like `m` get a sound, but more general type.
  var g = a.m(); // Succeeds, `g` has type `void Function(Never)`.
  g(1); // Compile-time error, `int` is not assignable to `Never`.
  if (g is void Function(int)) g(1); // OK
}

That proposal could be used to eliminate the rather aggressive behavior where it throws just because I'm accessing that member. However, we could just emulate that proposal manually by rewriting the program:

class C<X> {
  // The return type is now sound: a supertype of the returned object's type for any `X`.
  void Function(Never) get f => (X x) {};
}

My recommendation at this point would be to avoid having instance members with a contravariant type. This can be done by replacing the contravariant occurrences of class type variables by Never (in the example: we replace X by Never, because a parameter type is a contravariant position), and changing function types to Function if a class type variable occurs in a type variable bound of the contravariant type (so void Function<Y extends X>(Y) would become Function).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-as-intended Closed as the reported issue is expected behavior
Projects
None yet
Development

No branches or pull requests

3 participants