Site-www: Style and/or docs: AVOID contravariant function fields

Created on 21 Jul 2018 · 4Comments · Source: dart-lang/site-www

I think this is a big enough hurdle in Dart2 (especially from those used to Dart1) to prioritize. I've had to explain this separately to about 20 people this week, and not having anything good to point to has been a burden.

Related issues in the SDK:

I might have missed some. @munificent I'd be happy to pair with you on writing this rule.

Proposal

AVOID contravariant function fields

In Dart 2, class type arguments (i.e. List<T> are covariant[link to description]), while
function type arguments (i.e. bool Function(T)) are contravariant[link to description]).
This can lead to tricky behavior (and runtime cast exceptions).
class Animal {}
class Cat extends Animal {}

void main() {
  var cats = [new Cat()]; // List<Cat>
  List<Animals> animals = cats; // OK, covariance allows upcast safely (for reads).
}
You can get into a problem when you try to combine a covariant generic type with a
contravariant one. For example, imagine we want a Ecosystem<T>, where T defines
the type of living organism that lives there:
class Ecosystem<T> {}
And imagine you want this class to be useful to consumers without having to sub-class it to
provide functionality, so you decide to take a first-class _function_ to define some behavior,
such as a check if there is enough room for a new animal to join that ecosystem:
class Ecosystem<T> {
  final bool Function(T) canAdd;
  Ecosystem({this.canAdd});
}
This can run into very tricky situations, because an Ecosystem<Cat> _can_ be upcasted to
an Ecosystem<Animal>, but a Function(Cat) _cannot_ accept a Function(Animal)
[insert more explanation why]:
void main() {
  var catWorld = new Ecosystem<Cat>(canAdd: (Cat c) => /* TODO */ true);
  Ecosystem<Animal> animalWorld = catWorld; // OK
  animalWorld.canAdd; // TypeError: (Animal) => bool is not a type of (Cat) => bool
}
This is because of [insert more explanation why].

BAD
class Ecosystem<T> {
  final bool Function(T) canAdd;
  Ecosystem({this.canAdd});
}
GOOD

Ensure the user-visible function is a _method_ on the class, not a first class function:
class Ecosystem<T> {
  final bool Function(T) _canAdd;
  Ecosystem({bool Function(T) canAdd}) : _canAdd = canAdd;
  bool canAdd(T animal) => _canAdd(animal);
}
This works because the T animal above uses the same _covariant_ ... [insert more description].

GOOD

If you must make the user-visible type a function (i.e. you want the field to be mutable), use
bounded sound generics [insert better name of whatever this is actually called]. For example:
class Ecosystem<T> {
  bool Function<S extends T>(S) canAdd;
  Ecosystem({this.canAdd});
}
The S extends T ensures that the type S is ... [insert more description].

/cc @srawlins @leafpetersen @eernstg

e1-hours p2-medium

Source

matanlurey

👍1

Most helpful comment

Yes, I totally agree we need to do something to explain this issue once and for all in a canonical place we can point users to. (Perhaps the runtime error can even link to it.) I think the explanation is too big to cram into "Effective Dart", but maybe we can just create a standalone article for it.

Once I'm back from giving some training and more on top of other tasks, I can try to set aside some time to work on this.

Eventually, I hope we can fix this more directly by supporting static control over variance in generics. Covariant-everything was dubious but maybe reasonable with Dart 1's type system. With a strong type system, the whole value proposition shifts and it looks increasingly like a bad deal to me.

munificent on 23 Jul 2018

👍3

All 4 comments

Dart has unsafe covariant class generics. That has consequences.

We use the "covariant by generics" treatment of methods, where List<E>.add effectively has type void Function(Object) at run-time, and it performs a type check that the argument is an E, to delay some of that unsafeness until the covariant type is used (the method is called). We can only do that for method parameters, nothing else.

In particular, we can't do that for function-typed values anywhere. It's not fields that are the problem, you get the same issue with return types of method or function-typed arguments.
Example:

class C<T> {
  T value;
  C(this_value);
  void Function(T value) makeSetter() => (T value) { this.value = value; };
  void computeAndSet(T Function() f) { this.value = f(); }
}
main() {
  var i = C<int>(42);
  C<num> n = i;
  n.value = 42.0;  // fails at run-time (unsurprisingly)
  void Function(num) setter = n.makeSetter();  // fails at run-time.
  var setter = n.makeSetter();  // fails at run-time, because we infer the static type.
  n.computeAndSet(() => 42.0);  // fails at run-time.
}

So, the issue is with any function type in the API which mentions the type variable in a contravariant position, not just fields.

We also can't really avoid these cases (Map.putIfAbsent, Iterable.firstWhere(...orElse) etc, all take functions which return something of the collection's element/value type. Those are inherently unsafe, you need to use them at the correct type. They are also very convenient, so we can't avoid them completely (they exist on Iterable and Map!)

So, restricting return values (not just fields, fields just have getters with their type as return value) is probably a good recommendation, but not enough to completely avoid the issue. Restricting function typed arguments to no use the class type parameter in a contravariant manner is not viable.

As for the "good" example:

class Ecosystem<T> {
  bool Function<S extends T>(S) canAdd;
  Ecosystem({this.canAdd});
}

you are making canAdd a generic function. The argument function must be generic and have T as the exact bound, otherwise the generic function type is not assignable at all. If anything, this is more restrictive than the original non-generic function type.

lrhn on 22 Jul 2018

Once I'm back from giving some training and more on top of other tasks, I can try to set aside some time to work on this.

munificent on 23 Jul 2018

👍3

animalWorld.canAdd; // TypeError: (Animal) => bool is not a type of (Cat) => bool

We shouldn't have any caller-side checks at all. We also shouldn't _ignore_ the soundness issue, of course, but invariance (e.g., declaration-site invariance: dart-lang/language#214) could be used to eliminate the unsoundness.

If we give developers the ability to address this unsoundness issue at the root then we might be able to treat the occurrences that aren't fixed a more harsh treatment: Give these expressions a sound type (that is, a more general type than it has today, but one that the dynamic semantics will actually satisfy). Then there will be some additional downcasts or some newly failing member accesses (because that supertype doesn't have a particular member that the subtype had), but we could then eliminate the whole notion of caller-side checks.

eernstg on 14 Feb 2019

I removed the "EffectiveDart" tag because I think this is too deep of an issue to fit into the guidelines. I think it worth having a doc somewhere that explains how covariant generics work including the consequence of contravariance like the issue here. Over the long term, I would love to move away from unsound covariance in general once we have @eernstg's variance annotations.

munificent on 16 Dec 2020

Was this page helpful?

0 / 5 - 0 ratings