Handle covariant callbacks

[munificent]

Flutter has a few cases where they are trying to use functions in a way that is statically unsound. Almost all of them are around GestureRecognizer and look roughly like:

typedef GestureRecognizer GestureRecognizerFactory(GestureRecognizer recognizer);

Widget build(BuildContext context) {
  Map<Type, GestureRecognizerFactory> gestures = <Type, GestureRecognizerFactory>{};

  gestures[TapGestureRecognizer] = (TapGestureRecognizer recognizer) {
    ...
  };
}

Here, that map set isn't sound because the function literal takes a TapGestureRecognizer, not a GestureRecognizer. The former is a subtype of the latter, so this isn't statically safe.

Before Flutter can become strong mode clean, we need to address this. We need to decide how/if we want to change the language to handle code like this. And then, once we have, we need to implement and spec it.

[leafpetersen]

@sethladd @Hixie @abarth

[lrhn]

How does the return type of the factory function type vary?
Could the factory type be declared as:

typedef T GestureRecognizerFactory<T extends GestureRecognizer>(T recognizer);

or some other generic function type? Or is there no structure to how the argument and return type vary?

One solution I see is to just wrap the function in a class.

class GestureRecognizerFactoryBox<T> {
  final GestureRecognizerFactory<T> _factory;
  GestureRecognizerFactoryBox(this._factory);
  T create(T recognizer) => _factory(recognizer);
}

Then the class is correctly typed, and because it's a class type, not a function type, it allows covariant subtyping. That means that you can store a GestureRecognizerFactoryBox<TapGestureRecognizer> in the map and call it again without running into problems.
The create function has a contravariant occurrence of the type parameter, so we will insert a check, but that's expected for generic classes like that.

Alternatively, wrap the function when it's added to the map:

void addGestureRecognizerFactory<T extends GestureRecognizer>(Type<T> type, 
                                                              T factory(T recognizer)) {
  _map[type] = (GestureRecognizer r) => factory(r as T);
}

It sucks somewhat to have to make a class instance to keep a single function, but the existing code is type-unsafe as written, and we don't allow type-unsafe code. I don't think we should compromise that for code like this when there are simple workarounds.

Alternatively, don't use a plain normal map, use a specialized class with type-aware lookup and add operations which allow the specific type variations needed for handling what is basically dependently typed code.

[Hixie]

Could the factory type be declared as:

typedef T GestureRecognizerFactory<T extends GestureRecognizer>(T recognizer);

Yes, that would be an accurate description of the constructors. The reason we don't do that is that there's no way to specialise the type per entry in the map, so there's not much gain to be had from it.

It sucks somewhat to have to make a class instance to keep a single function, but the existing code is type-unsafe as written, and we don't allow type-unsafe code. I don't think we should compromise that for code like this when there are simple workarounds.

The ability to write this kind of code is part of what made Dart 1.0 great. I would be very sad to lose it.

[munificent]

Alternatively, wrap the function when it's added to the map:

void addGestureRecognizerFactory<T extends GestureRecognizer>(Type<T> type, 
                                                              T factory(T recognizer)) {
  _map[type] = (GestureRecognizer r) => factory(r as T);
}

Yup. I fleshed out a proof of concept of that here: https://github.com/munificent/flutter/commit/8def6c700d996efd1aa3c81be620555bf6001529

[leafpetersen]

status: under discussion.

[Hixie]

I think if we had true generic methods (i.e. generic methods were at runtime we could inspect the type argument) then we could rewrite this code to not require covariant callbacks.

[floitschG]

Not planned for 1.50.

[Hixie]

@cbracken ran into a case today where he wanted to say covariant in a typedef.

[srawlins]

cc @matanlurey we've discussed this one a lot 😄

[eernstg]

covariant in a typedef does not fit well:

With covariant on a formal parameter declaration in an instance method, we will get dynamic checks for the actual argument, and we get the permission to add further covariance on the same parameter in all subtypes.

In order to ensure expression soundness we give a tear-off of such a method the reified parameter type Object for each parameter which is covariant. In other words, we allow such a tear-off to masquerade as a function that accepts everything for that parameter.

However, we did not allow covariant as part of a function type (say, in a typedef) at all: Among other things, a function type is used as a type annotation in declarations of variables, parameters, returned values, etc. In all those cases the relevant function object exists already. This means that we cannot retroactively change its dynamic type (short of creating a wrapper, which is an anomaly because the original function and the wrapper would have to have different identity, because they have different types, and assignment doesn't otherwise create a new object). I believe that it's just not a good idea to try to make covariant a property of the type of a function.

However, we could certainly allow functions in general (including function literals) to obtain the same kind of flexibility, using the same syntax:

void globalFunction(covariant int x) => x.isEven; // Safe, will check that `x is int`.
void Function(dynamic) f = globalFunction as dynamic; // OK, is `void Function(Object)`.

main() {
  f = (covariant int x) => x.isEven; // Also fine, is `void Function(Object)`.
  globalFunction('Not an int'); // Compile-time error.
  ((covariant int x) => x.isEven)('Not an int'); // Compile-time error.
}

The underlying principle is that function types (static and dynamic) have no idea that any particular function has a covariant parameter, it's completely an implementation detail for that function: It announces (in its dynamic type) that it will accept arbitrary arguments (Object), but then it will check dynamically that it actually gets something more specific (here: int), and the body of the function can then use that more specific type. But the rest of the world will not be able to see these things, they just see a function accepting an Object, according to the dynamic type.

The static type preserves the declared type of the covariant parameter. This basically prevents "stupid" invocations where we can predict that the dynamic check will fail (like globalFunction('Not an int')).

So if you actually want covariant to be part of a function type then we'd need to reconsider the entire story about what a covariant parameter is. I'm not saying that this is impossible, just that it's pretty far away from the thinking that we used when this feature was introduced.