Weird not a subtype of issue

[lukehutch]

[Edit by eernstg: This issue has evolved over time; it is now used to report that diagnostics emitted for run-time failures associated with caller-side checks are uninformative.]

Sorry for the non-specific title, I don't even know how to describe this weird bug...

Also, I will illustrate this with Flutter code, but this is probably not a Flutter issue, it is a type system issue as far as I can tell...

Given this code:

enum Range {
  local,
  global,
}

void main() async {
  runApp(
    MaterialApp(
      home: Scaffold(
        body: SafeArea(
          child: ValueButton<Range>(   // (*) ValueButton defined below
            value: Range.global,
            label: 'Global',
            onChanged: (range) => print(range),
          ),
        ),
      ),
    ),
  );
}

if I define ValueButton as a StatelessWidget, as follows

class ValueButton<T extends Enum> extends StatefulWidget {
  final String label;
  final T value;
  final void Function(T?) onChanged;

  const ValueButton(
      {required this.label,
      required this.value,
      required this.onChanged,
      super.key});

  @override
  State<ValueButton> createState() => _ValueButtonState();
}

class _ValueButtonState<T extends Enum> extends State<ValueButton<T>> {
  @override
  Widget build(BuildContext context) {
    return ElevatedButton(
        onPressed: () => widget.onChanged(widget.value),     // (*) Runtime error (line 69 in error below)
        child: Text(widget.label));
  }
}

then when I click on the button, I get:

════════ Exception caught by gesture ═══════════════════════════════════════════
The following _TypeError was thrown while handling a gesture:
type '(Range?) => void' is not a subtype of type '(Enum?) => void'

When the exception was thrown, this was the stack
#0      _ValueButtonState.build.<anonymous closure> main.dart:69
[...]

However if I convert ValueButton to a StatelessWidget

class ValueButton<T extends Enum> extends StatelessWidget {
  final String label;
  final T value;
  final void Function(T?) onChanged;

  const ValueButton(
      {required this.label,
      required this.value,
      required this.onChanged,
      super.key});

  @override
  Widget build(BuildContext context) {
    return ElevatedButton(
        onPressed: () => onChanged(value), child: Text(label));    // (*) No runtime error this time
  }
}

then I get no such error:

I/flutter ( 9919): Range.global

The only difference here is StatefulWidget vs. StatelessWidget.

--

• Dart SDK Version (dart --version): 3.0.5
• Whether you are using Windows, MacOSX, or Linux (if applicable): Linux

[eernstg]

It might help to use onPressed: () => (widget as dynamic).onChanged(widget.value) rather than onPressed: () => widget.onChanged(widget.value).

The point is that it is likely that widget uses covariant subtyping (so the statically known type is ValueButton<Enum> but the run-time type is ValueButton<Range>), and then you get a caller-side check when widget.onChanged is evaluated (you don't even get to the point where it is called).

Here is the core typing issue:

enum Range { local, global }

class ValueButton<T extends Enum> {
  final T value;
  final void Function(T?) onChanged;
  const ValueButton(this.value, this.onChanged);
}

void main() {
  ValueButton<Enum> button = ValueButton<Range>(Range.local, (range) {});
  button.onChanged; // Throws, already when we touch it (no need to call it).
}

I'd recommend that you do not declare an instance variable whose type contains a type parameter in a non-covariant position (see this issue about "contravariant members").

It's no problem for code in the scope of the relevant type variable (such as code in the body of the ValueButton which is a StatelessWidget), but it causes run-time errors when a contravariant member is used from the outside (if and only if the receiver has a static type whose type argument is different from the run-time value of that type parameter).

The typical way to get this issue is that a type parameter is used as the type of a value parameter in a function type, here: void Function(T?). There are other positions which are non-covariant, but this one covers the vast majority of cases.

So the advice is: Do not declare an instance variable with type SomeReturnType Function(... T ...) where T is a type variable declared by the enclosing class (T? is included, and List<T>, etc).

The reason why it helps to make the invocation dynamic is that this allows you to evaluate widget.onChanged without encountering a run-time type error, and then you're actually calling the function with an argument that has the required type.

So you should probably just do the following, in order to avoid touching onChanged from the outside:

class ValueButton<T extends Enum> extends StatefulWidget {
  final String label;
  final T value;
  final void Function(T?) onChanged;

  const ValueButton(
      {required this.label,
      required this.value,
      required this.onChanged,
      super.key});

  @override
  State<ValueButton> createState() => _ValueButtonState();

  void callOnChanged() => onChanged(value);
}

class _ValueButtonState<T extends Enum> extends State<ValueButton<T>> {
  @override
  Widget build(BuildContext context) {
    return ElevatedButton(
        onPressed: () => widget.callOnChanged(),
        child: Text(widget.label));
  }
}

[lukehutch]

Thank you for the extensive explanation. And your suggested fix worked.

Can the error message be improved here? I never would have figured this out without your help.

[lrhn]

Marking as front-end, in case they can do something general to improve the error message.
If not, there needs to be an issue for each individual backend.

[eernstg]

Well, I've proposed a lint (tentatively called unsafe_variance) here: #59050. It would flag declarations like final void Function(T?) onChanged because it uses T in a parameter type (in general: because a getter or method return type has a non-covariant occurrence of a type variable of the enclosing class or mixin).

It would probably not be easy to give advice about what else to do in this situation. However, indicating that this particular kind of declaration is dangerous would in any case be a useful first step.

The lint proposal says that a getter/method return type R like void Function(T?) where T is a type variable should simply be replaced by a type which is covariant/invariant and known to be a supertype of R. That is, it recommends that the type is changed to something that provides a smaller amount of information, but all of it is true.

In this case that could be void Function(Never); call sites would then have to call onChanged dynamically (because they probably aren't going to pass an argument whose static type is Never), or they can test the function type as mentioned in #59050. This is not convenient, but it is a perfectly valid characterization of the situation: We're calling a function in a way that may or may not work. With that in mind, we might also consider declaring it as final Function onChanged;, which is honestly untyped.

[Bungeefan]

Sorry if this is the wrong place, because I am not entirely sure if it belongs here.
If not, I will be happy to open another issue.

I came across a very similar error and was able to reproduce it with the following snippet.
This produces a runtime TypeError:
type '(int) => int' is not a subtype of type '((num) => num)?'

typedef Updater<T> = T Function(T initialValue);

abstract class SuperTest<T> {
  final Updater<T>? updater;

  SuperTest(this.updater);
}

class Test extends SuperTest<int> {
  Test(super.updater);
}

void main() {
  Test test = Test((initialValue) => initialValue);
  // var superTest = test as SuperTest; // breaks too
  var superTest = test as SuperTest<num>;
  var returnTest = superTest.updater;
  print(returnTest);
}

[eernstg]

Yes, this situation is similar. SuperTest declares an instance variable whose type is contravariant in the type variable T, namely Updater<T>? (which means T Function(T)?). Hence, this is yet another case where a class has a "contravariant member", as described in this issue: dart-lang/language#297.

It is dangerous to use a class with a contravariant member, because the program will incur a dynamic error (it throws a _TypeError) if that member is evaluated and the dynamic type has a more special type argument than the static type (you don't even have to call it, just reading a contravariant variable or tearing off a contravariant method will throw). (In this case we had a Test which is a SuperTest<int> with static type SuperTest<num>, and int is more special than num).

You could vote for this lint if you want to get a heads-up whenever a class has that kind of member: #59050.

You would be able to eliminate the covariance as follows (this will turn the run-time error into a compile-time error):

// Assuming `--enable-experiment=variance`.

abstract class SuperTest<inout T> {
  final Updater<T>? updater;

  SuperTest(this.updater);
}

class Test extends SuperTest<inout int> {
  Test(super.updater);
}

The above code relies on a feature which hasn't yet been added to the language, namely declaration-site variance, dart-lang/language#524 (you can vote for that, too ;-).

The modifier inout on a type parameter specifies that it can be used everywhere (including as a parameter type in the type of an instance variable). In return for that flexibility, the class that has this type parameter is invariant in that type parameter (this means that SuperTest<int> and SuperTypes<num> are unrelated types, neither of those are assignable to the other).

We don't have that feature at this time, but you can also emulate it:

typedef Updater<T> = T Function(T initialValue);

typedef SuperTest<T> = _SuperTest<T, Function(T)>;

  abstract class _SuperTest<T, Inv> {
  final Updater<T>? updater;

  _SuperTest(this.updater);
}

class Test extends SuperTest<int> {
  Test(super.updater);
}

void main() {
  Test test = Test((initialValue) => initialValue);
  SuperTest<num> superTest = test; // Compile-time error.
  var returnTest = superTest.updater;
  print(returnTest);
}

Note that the original code var superTest = test as SuperTest<num>; is still allowed, and it is still unsafe. That's simply because as is inherently unsafe. It tells the compiler that you know better (and it's your problem if you don't actually know better ;-).

That's just like int i = true as int;: The compiler is happy, but it will throw because true isn't an int. So don't use as if you care about static type safety.

[lukehutch]

I think I just ran into this issue again, in a different context: I got the runtime exception:

type '(BuildContext, ProfileResult, int) => ProfileThumb' is not a subtype of type '(BuildContext, dynamic, int) => Widget'

Here, ProfileThumb is a subtype of Widget, and I would have expected ProfileResult to considered a "subtype" of dynamic.

I'm actually highly surprised this code has an error, let alone a runtime error.

The problem was I forgot to add type parameters to an enclosing class. (The actual code details don't matter, presumably.)

However, I don't see how there would be any harm in letting this sort of error slide at runtime -- how could calling a function with a dynamic type parameter with a specifically-typed parameter ever fail?

[lrhn]

However, I don't see how there would be any harm in letting this sort of error slide at runtime -- how could calling a function with a dynamic type parameter with a specifically-typed parameter ever fail?

The problem is that you don't have a function with a dynamic-typed parameter. You have a function with a parameter type ProfileResult, which you are trying to cast to a function type with dynamic as parameter type.
That cannot be allowed. The dynamic-parameter function type can be called with a String as argument, the function value you actually have cannot. It's not substitutable, so it's not a subtype.

[lukehutch]

I just ran into this issue yet again, and it took me a full half hour to figure out that this is the same issue... it would really help if, at a minimum, the linter could warn about this. Hitting a type error at runtime for something that on the surface seems correct is jarring (not to mention it decreases runtime safety, which is never a good thing, especially for a statically-typed language...).

Is there any way for the compiler to check for this error at compiletime?

[eernstg]

Is there any way for the compiler to check for this error at compiletime?

Certainly. It's a deliberate choice that Dart uses dynamically checked covariance. Statically checked covariance is possible, and it's safe, but it is considerably less convenient in a lot of situations where there are no run-time failures. Still, I'd very much support the introduction of statically checked variance in Dart such that developers can make the choice.

The lint proposed in #59050 would inform us that it's a bad idea in the first place to have an instance variable whose type is void Function(T?). That's an important step, because such types are very, very likely to cause run-time errors.

Alternatively, the declaration-site variance feature in dart-lang/language#524 would allow us to specify that ValueButton is invariant in its type argument. This would make it an error to assign a ValueButton<T> to a variable of type ValueButton<S> except when S and T are subtypes of each other (so, roughly, it's the same type):

void main() {
  ValueButton<Enum> button = ValueButton<Range>(Range.local, (range) {}); // Compile-time error.
  ...
}

This is considerably less convenient for the program as a whole, so that's the trade-off. On the other hand, when ValueButton is invariant in its type parameter T, it's perfectly safe to have an instance variable with type void Function(T?).

Finally, if that is possible, you could limit the issue to the library that declares ValueButton by making the "contravariant instance variable" private, and then perform the invocation in a method of ValueButton:

class ValueButton<T extends Enum> extends StatefulWidget {
  final String label;
  final T value;
  final void Function(T?) _onChanged; // DANGER: Only use from this class body.

  const ValueButton(
      {required this.label,
      required this.value,
      required this.onChanged,
      super.key});

  @override
  State<ValueButton> createState() => _ValueButtonState();

  void onChanged(T? value) => _onChanged(value);
}

This might look like it makes no difference compared to the approach where onChanged is an instance variable, but the difference is that you can call the onChanged method on an instance myValueButton of type ValueButton<Range> even in the case where it has a static type which is ValueButton<T> for some T which is a supertype of Range. It only throws if the given value doesn't have the required type. In contrast, myValueButton._onChanged will throw in that situation, and you never even get far enough to try to call it.

[lukehutch]

Thanks for expanding your explanation further.

I would actually be fine with the linter or compiler requiring instance variables to be private, if an attempt is made to invoke function-typed covariant instance variables in this way. Adding a function to access the instance variable had worked perfectly in every case I have run into so far.

I suspect that very few programmers will be able to wrap their heads around this issue (I'm still struggling, to be honest... I need a deeper understanding of covariance and contravariance) -- so really the programmer needs to be told exactly how to fix this issue when it comes up. I would be fine with none of the deeper changes to the language that you suggested being made, if I was told directly every time that I can only invoke these types of instance variables' function values from a member function.

[eernstg]

@lukehutch, I think we can close this issue. The observed behavior is as intended, and the way to improve on the difficulties caused by this behavior is to report "contravariant members" (which is requested by #59050) and/or introduce statically checked variance (as proposed in dart-lang/language#524). WDYT?

[lukehutch]

Sure, although I will point out again my suggestion in my previous comment that the source of the problem needs to be explained more clearly in the error message. (Dart's error messages are usually very helpful and information-rich, but not in this case.) Can this be fixed? If not, please go ahead and close this.

[eernstg]

(@johnniwinther, the text below contains a question for you—search for "@johnniwinther" to see it.)

@lukehutch wrote:

the source of the problem needs to be explained more clearly in the error message.

Ah, I'm sorry! I forgot that the error message was actually the primary topic of this issue at this time. I was just thinking about the hard typing properties of the examples. ;-)

The error message which was reported in the initial posting was something like the following:

type '(Range?) => void' is not a subtype of type '(Enum?) => void'

This error message describes the soundness violation which was just about to take place. So the runtime threw a TypeError because that's what it does in that situation, rather than allowing the soundness violation to occur.

The description in the error message is precise, but it is also superficial. This is because it reports the immediate problem at the time where it exists, and there is no information about the sequence of events that caused this soundness violation to be lined up in the first place.

It seems possible to track down the previous step when the upcoming soundness violation is detected: The type check was performed because line 69 in the original example would evaluate widget.onChanged in order to get hold of a function object and call it, and the resulting function object does not have the statically known type.

It is known at compile time that widget.onChanged can do this, which is the very reason why the compiler generates code to check the actual type at run time (this is known as a 'caller-side check', and it is used in exactly the situation where a contravariant member is being evaluated).

@johnniwinther, do you think it would be difficult to report the fact that any given run-time type failure is caused by a failing caller-side check? (The error message could perhaps give a link to a page about "contravariant members".) It could be a useful heads-up for developers whose code have this kind of member declarations that the typing of the member has caused this run-time type error.

However, it could also be argued that the contravariant member declaration should be reported as highly questionable at compile time. That would simply be a matter of getting the lint proposed in #59050 implemented, and enabling it (preferably: as widely as possible).

On the other hand, it could be argued that the contravariant member onChanged isn't so bad in its own right, we just need to give developers a heads-up if they ever use it on a different object than this.

That is exactly the approach which is used if we add a method like callOnChanged (as shown here), except that we aren't supporting this particular discipline by any kind of enforcement, we are simply changing the concrete usage of onChanged from widget.onChanged(widget.value) to onChanged(value) (that is: this.onChanged(this.value)), where the former is unsafe and the latter is safe.

We could introduce a feature like "members that are private to this" and turn it into a compile-time error to access a member like onChanged on any other receiver than this (that is, we can do this.onChanged..., and we can do onChanged..., when it means the same thing because it implicitly uses this as the receiver).

class A<X> {
  void Function(X) this.onChanged; // Strawman syntax `this.` means "private to `this`".
  X value;
  A(this.onChanged, this.value);
  void foo() { onChanged(value); } // OK.
}

void main() {
  A<num> a = A<int>((i) => i.isEven, 42);
  a.onChanged; // Compile-time error, receiver is not `this`.
  a.onChanged(a.value); // Just another example of the same error.
}

This kind of feature could allow us to turn any contravariant member into a type safe entity: As long as it is only accessed on this, the type variables in its type are in scope, and usages can be type checked statically. No problem.

However, as long as we don't have this kind of feature (or anything else that will do the job), we have to consider techniques like the addition of callOnChanged as delicate.

That is, such techniques are OK for developers who know exactly what they are doing (and who are willing and able to remember how to deal with contravariant members like onChanged also when they need to maintain the code a few months or years later ;-). To me, this implies that it is not a problem to have a standard response which is to warn about contravariant members because they are inherently unsafe. The few developers who really need to do delicate things can // ignore: the warnings.

I think the conclusion is that (1) it is probably not too hard to modify the error message which is associated with a failing caller-side check, but (2) the fact that techniques like callOnChanged have been used to make sure that an otherwise unsafe contravariant member is only used in a safe manner is probably beyond static analysis, developers who do this will just need to be careful (that is, unless we get a feature like "private to this").

[johnniwinther]

The AsExpression node produced by the CFE to perform the check has a isCovarianceCheck flag that is set for this case, so it should be possible for backends to produce a specific error.