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

Allow covariant to be used for arbitrary function types? #477

Open
jamesderlin opened this issue Jul 29, 2019 · 5 comments
Open

Allow covariant to be used for arbitrary function types? #477

jamesderlin opened this issue Jul 29, 2019 · 5 comments

Comments

@jamesderlin
Copy link

Currently this is rejected by dartanalyzer:

void Function(covariant dynamic) foo = (int x) {};

which complains:

error • The 'covariant' keyword can only be used for parameters in instance methods or before non-final instance fields at ... • invalid_use_of_covariant
error • A value of type 'Null Function(int)' can't be assigned to a variable of type 'void Function(dynamic)' at ... • invalid_assignment

I admit that I have little experience with covariance, but why is covariant restricted to instance methods? What is intrinsically safer about instance method overrides when the covariant type isn't related to the instance's class?

Motivation:

A common redux pattern is implementing a reducer like:

State reducer(State state, dynamic action) {
 if (action is FooAction) {
   return foo(state, action);
 } else if (action is BarAction) {
   return bar(state, action);
 } else if (action is BazAction) {
   return baz(state, action);
 }
 return state;
}

State foo(State state, FooAction action) {
 ...
}

State bar(State state, BarAction action) {
 ...
}

I instead want to store this in a Map<Type, State Function(State, dynamic)> so it can be used like:

State reducer(State state, dynamic action) {
  final handler = reducerTable[action.runtimeType];
  if (handler != null) {
    return handler(state, action);
  }
  return state;
}

This works, but it has the disadvantage that the handlers must all be declared with dynamic action instead of with more specific types:

State foo(State state, dynamic _action) {
  FooAction action = _action as FooAction;
  ...
}

State bar(State state, dynamic _action) {
  BarAction action = _action as BarAction;
  ...
}
@matanlurey
Copy link
Contributor

The covariant keyword means "covariant to/for subtypes of this class". A top-level function doesn't have a class type, and thus the word covariant would not apply to anything. It seems to me you could define them using extends?

State reducer<A extends Action>(State state, A action) {
  ...
}

FWIW, I think the design pattern above is a terrible fit for Dart (it relies on dynamic dispatch, lots of runtime checks, storing untyped functions in Maps, etc). I have seen a lot of APIs that look (and likely perform) a lot better for Dart/Flutter.

@eernstg
Copy link
Member

eernstg commented Jul 30, 2019

Agreeing with @matanlurey that this is not a good fit, I'd add a couple of extra considerations.

The language team has had discussions about allowing covariant on parameters of functions that aren't instance methods, cf. dart-lang/sdk#27487.

The semantics would be (1) the reified type of the function would use a top type (Object today, soon to be Object?) for that parameter, and (2) at the beginning of the body of the function a dynamic check would be performed. For instance:

void foo(void Function(Object) g) {
  g(42); // Dynamic check for invocation from `main` does succeed.
}

main() {
  foo((covariant int i) => print(i.isEven));
}

The point is that this allows the function body to be checked using the type int for i (so the code is checked more strictly than it would have been with (dynamic i) => ...), and the reified type of the function object passed to foo satisfies the requirement void Function(Object). So we're in a sense redistributing the dynamism to be focused at that single point where it is checked that i is int.

The original example would use it as follows:

State foo(State state, covariant FooAction action) {
 ...
}

State bar(State state, covariant BarAction action) {
 ...
}

However, putting covariance into function types is a different matter; see this comment for some reasons why that would probably be a bad idea.

However, you could also just switch to use an actual subtype relationship: Use a Map<Type, State Function(State, Null)> to store those functions (you'll have to replace Null by Never when we add non-null types, but the idea is that this parameter type should be a subtype of every possible parameter type, which makes the function type a supertype of those with other parameter types, because function types are contravariant in their parameter types). Such a map would happily hold functions of type State Function(State, T), no matter what T stands for, both with respect to the static checks and any dynamic checks that may exist (e.g., due to downcasts from dynamic).

You'd then have to call the function dynamically (in order to avoid a failure due to a dynamic check based on the statically known parameter type Never):

State foo(State state, FooAction action) {...} // No special tricks here.
State bar(State state, BarAction action) {...} // Ditto.
...

State reducer(State state, dynamic action) {
  final handler = reducerTable[action.runtimeType];
  if (handler != null) {
    return (handler as Function)(state, action);
  }
  return state;
}

[Edit: Avoid the type case in reducer, cf. this comment by @jamesderlin.]

#295 is concerned with a change whereby you would be able to use a plain foo(state, action) to call them. But even with (foo as Function)(...) it would only affect reducer, and the rest of the code would have suitably strict types.

@jamesderlin
Copy link
Author

@matanlurey:

FWIW, I think the design pattern above is a terrible fit for Dart

Do you mean that the Map-based approach is substantially worse than the original if (action is ...) chain, or that they're both bad?

@eernstg: Thanks! I presume that your last example was meant to be:

State reducer(State state, dynamic action) {
  final handler = reducerTable[action.runtimeType];
  if (handler != null) {
    return (handler as Function)(state, action);
  }
  return state;
}

because (foo as Function), (bar as Function), etc. weren't necessary in the original if-chain form.

@matanlurey
Copy link
Contributor

@matanlurey:

FWIW, I think the design pattern above is a terrible fit for Dart

Do you mean that the Map-based approach is substantially worse than the original if (action is ...) chain, or that they're both bad?

Both IMO. This would be totally just bypassing Dart's type system.

I'd consider using something like the Visitor pattern instead of dynamic registration of handlers.

@eernstg
Copy link
Member

eernstg commented Jul 31, 2019

@jamesderlin wrote:

I presume that your last example was meant to be: ...

That's right, thanks! Using the concise reducer is of course crucial in the likely case that FooAction, BarAction, ..., are declared in libraries that are not imported by the library that declares reducer.

That allows for an architecture which is extensible with respect to the kinds of actions supported, because actions can be added without editing reducer, given that the author of each kind of XAction is required to add an XAction entry to reducerTable. There'd be some kind of registration step at startup, cf. #371.

@matanlurey wrote:

This would be totally just bypassing Dart's type system

I agree that this technique involves some elements which are potentially costly in terms of static analyzability.

In particular, using .runtimeType may cause the program size to grow and performance to drop, because it causes a larger set of actual type arguments to be reified. (Otherwise, if it can be shown for a class C<X> .. that the actual value of X is never needed at run time then instances of C might be compiled to a form where there is no representation of that value at run time). However, that issue could be eliminated by introducing support for obtaining the run-time class of a given instance (say, as a static method Type.classOf(Object o)), which will not use the actual type arguments (e.g., the run-time class of a List<int> would be List, represented as List<dynamic> or as some new representation of a class-without-type-arguments).

However, the use of a Map<Type, State Function(State, Never)> is disciplined in such a way that it could have been described as a dependent type: Map<t: Type, State Function(State, t)>. This is hypothetical syntax, Dart doesn't have dependent types, but it would mean "for each entry of this map, there is a key t of type Type; let T be the type reified by t, then the corresponding value is of type State Function(State, T).

We won't have dependent types in Dart, so we cannot rely on modeling the situation using dependent types.

But I think it makes sense to consider the trade-off between benefits like having an architecture which is extensible in certain ways, at the cost of emulating a new kind of types (which requires some manual discipline, because the type checker can't see it), versus having an architecture which is less extensible (here: we must edit some core framework code like reducer each time we add a new kind of action; or we must edit each visitor and add an extra method to handle the new kind of action). I think this trade-off is a bit more nuanced than just choosing between a statically or a dynamically typed coding style.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants