isAssignableTo is not symmetric, but used in some checks where that is expected

[jmesserly]

Strong mode isAssignableTo is almost equivalent to:

isAssignableTo(t1, t2) iff t1 <: t2 or t2 <: t1 

But there's an extra check to rejecting an implicit cast from a function type to a callable class. There's also an optional flag to disable implicit casts. (and there's dead code relating to generic functions, which isn't reachable anymore because we don't allow generic/non-generic functions to subtype each other)

For newer features (like parameters marked with the covariant keyword) we started using t1 <: t2 or t2 <: t1.

It might be nice to have a symmetric "is either of these a subtype of the other" operation, and a different operation for checking if assignment (and potentially an implicit downcast) should be allowed.

At the very least, we should clean out the dead code from isAssignableTo :)

[lrhn]

Do you know why there is a check rejecting implicit cast from a function type to a callable class?
It's possible that the function is an instance of the class, so I don't see any reason to single out that implicit cast compared to other similar implicit down-casts.

It sounds like it's the only actual difference between isAssignableTo and <: || :> (if you ignore the flags to disable implicit casts, which is a lint, not part of the language).

[eernstg]

I was wondering so I checked a couple of examples: Strong mode assignability actually does seem to use "S <: T || T <: S" assignability, also for function types. For example:

typedef SuperFun = int Function(int);
typedef SubFun = int Function([int]);

void foo(SubFun f) => f();

main() {
  SuperFun f = (int n) => n;
  foo(f);
}

// This produces the following response (no hints: unused variable):
//
//   > dartanalyzer n032.dart
//   Analyzing n032.dart...
//     warning • The argument type '(int) → int' can't be assigned to the parameter type '([int]) → int' at n032.dart:22:7 • argument_type_not_assignable
//   1 warning found.
//   > dartanalyzer --strong n032.dart
//   Analyzing n032.dart...
//   No issues found!

It's slightly surprising that strong mode is more forgiving in this particular kind of situation. Of course, we can outlaw the downcast with --no-implicit-casts to make strong mode less forgiving, but we could have done that in Dart 1 as well.

The reason why Dart 1 was designed to be more strict in this particular place was probably the run-time effect: Even if you ignore type annotations completely, the function which gets called at run time with f() does not support invocations with zero positional arguments.

The obvious question is then: Should we preserve the approach in strong mode whereby function type assignability admits upcasts and downcasts, but it does not admit assignments where the structure of the argument list of the assigned value violates the requirements according to the type annotation of the assignee? I guess it could be implemented as an additional "argument list shape check" which is performed on function-type-to-function-type downcasts in addition to current assignability checks.

Of course, this would mean that assignability is still not symmetric, but I guess the implementation would just need to do the right thing when we have resolved what that is. ;-)

[jmesserly]

Do you know why there is a check rejecting implicit cast from a function type to a callable class?
It's possible that the function is an instance of the class, so I don't see any reason to single out that implicit cast compared to other similar implicit down-casts.

I'm not sure! Perhaps @leafpetersen remembers?

[leafpetersen]

The reason why Dart 1 was designed to be more strict in this particular place was probably the run-time effect: Even if you ignore type annotations completely, the function which gets called at run time with f() does not support invocations with zero positional arguments.

I have no idea what the thinking behind the Dart 1 function behavior was. Given the general philosophy of "if it could potentially work we should allow it", I would expect this to be allowed in Dart 1 (since the thing assigned to SuperFun could have an optional argument). But it's not, so... shrug.

I'd be fine with changing function type assignability to strict subtyping.

The obvious question is then: Should we preserve the approach in strong mode whereby function type assignability admits upcasts and downcasts, but it does not admit assignments where the structure of the argument list of the assigned value violates the requirements according to the type annotation of the assignee?

I'm not really following this. I don't believe that there are any additional checks that you can add that don't make currently working code into a static error, are there? I'm obviously in favor of not allowing the implicit downcast, but I feel like adding additional ad hoc checks here just makes things confusing. I'm a type theorist, and I still have to go and re-read the Dart 1 spec and scratch my head at length to figure out what is and is not supposed to be allowed for function types under the Dart 1 spec. For example:

• Subtyping definition in the spec doesn't match intended (and implemented) behavior #22033
• Dart has semi-recursive types. Should it? #29791 (comment)

So basically, I'd be fine with only doing one way subtyping here, but I don't think I'm keen on something in between. But I suppose I could be convinced otherwise if there's a reasonably understandable point in the middle.

[leafpetersen]

I'm not sure! Perhaps @leafpetersen remembers?

I think we basically observed that function types are almost never inhabited by instances of nominal types at runtime, so this implicit cast was essentially always going to fail, so why not make it an error? The user can always put an explicit cast if they're sure that it's the right thing.

[lrhn]

That can be said for many implicit downcasts - you can always just cast explicitly if you mean it, and this particular case sounds less error-prone than many other ones that we do allow.
If you are casting a function type to a class type, and the class type is callable with a compatible function signature, then I'd expect the cast to be deliberate more often than not. There aren't that many callable classes anyway.

I think it's an exception (and therefore complication) that doesn't feel like it's carrying its own weight.
If accidental downcasts to callable classes actually happens often, I'd probably reconsider, but otherwise I'd just remove that restriction.

[eernstg]

Having thought about this a bit more, I think it's reasonable to drop the distinction: We used to treat an argument list shape mismatch as a more serious fault than a type annotation mismatch, but with the Dart 2 run time discipline (heap soundness & expression soundness) the type annotation mismatch will lead to a dynamic error just as inevitably as the argument list shape mismatch.

With that, using a plain "S <: T || T <: S" definition of assignability also for function types would be a good choice. On top of being meaningful, it's also a simplification.

I agree with Lasse that it is a somewhat weakly motivated exception to treat downcasts from a function type to a callable class differently from downcasts between function types. Presumably, such a downcast would occur in the source code exactly in those situations where developers have a good reason to expect that it will actually succeed.

If the phrase "almost equivalent" doesn't hide anything extra, dropping this exception would make assignability completely symmetric. That would be a simplification of Dart as a whole: Developers will need to learn about assignability at some point, but if the story about assignability is short and sweet then they can continue doing their actual work almost immediately. ;-)

Allowing or disallowing implicit downcasts is a separate topic, so let's take that somewhere else.

[leafpetersen]

This is a bit of a digression from the subject at hand, but I am not willing to let this slide by, because it keeps coming up:

Presumably, such a downcast would occur in the source code exactly in those situations where developers have a good reason to expect that it will actually succeed.

I have no idea why you would presume this, and I think it reflects a fundamental misunderstanding of the way that most code comes into being. In particular, most code comes into being through editing, modification, and refactoring of already written code. So we start with code that looks like this (hugely simplified from a real code base, of course):

typedef int Callback(int x);

class Foo {
   Callback c;
   void registerCallback(int f(int x)) { c = f; };
}
void main() {
  new Foo().registerCallback((x) => x);
}

And now we come along and decide we want to refactor the code to make Callback a callable class:

abstract class Callback {
  int call(int x);
}

class Foo {
   Callback c;
   void registerCallback(int f(int x)) { c = f; };
}
void main() {
  new Foo().registerCallback((x) => x);
}

And we get no static feedback that our code is now broken. We didn't have a good reason to expect the implicit cast would succeed because we had no idea an implicit cast was happening. They are implicit. That's the whole point. There is no syntactic difference whatsoever between code with and without an implicit cast, and therefore to "intend" the implicit cast, we have to fully simulate, in our head, the type checking algorithm to understand where all of the implicit casts are happening.

In my experience, most people are not aware of places where implicit casts are happening at all. The IDE tells them the places where invalid assignments are happening, and all other places are assumed good until proven otherwise.

I have had numerous issues filed, and feedback provided, to the effect that refactoring is hard in Dart because of implicit casts.

I have had no issues filed that I can recall, or any other feedback provided, to the effect that having to explicitly downcast from a function type to a nominal type was a problem.

I actually think it would be a fascinating user study to pick a large range of recent CLs, find all of the implicit casts added by that CL, and ask the author of the CL whether they intended/were aware of, the implicit cast that was added by their code. @lukechurch

[leafpetersen]

Concretely to the main point of this issue. We were not able to remove call methods from the Dart 2.0, so my plan to solve the issue described here: #29791 is to remove call method subtyping from the type system, and instead allow assignment to introduce an implicit tear-off of the call method when assigning from a callable class to a function type. This means that the asymmetry will remain, since there is no coercion in the other direction, and there is no subtyping relation between function types and callable classes.

[eernstg]

This is a bit of a digression from the subject at hand, but I am not willing to let this slide
by, because it keeps coming up:

Presumably, such a downcast would occur in the source code exactly in those situations
where developers have a good reason to expect that it will actually succeed.

I have no idea why you would presume this, and I think it reflects a fundamental
misunderstanding of the way that most code comes into being.

It sounds a little bit like you're addressing implicit downcasts in general here, not the asymmetry which is the topic of this issue. The point I was making was relative to downcasts in general: If we assume that implicit downcasts are being managed in a reasonable way then I would expect implicit FunctionType-2-CallableClass downcasts to be no more dangerous than other implicit downcasts. Btw, I believe this is very much in line with Lasse's argument on the same topic.

Of course, I already advocated giving programmers some more help in the area of implicit casts, say, by having IDEs give every expression which is subject to an implicit downcast a different background color, and I'm also open to have more of --no-implicit-casts by default. That's just a different topic.

In particular, most code
comes into being through editing, modification, and refactoring of already written code.

Right, and that's a very good reason to think carefully about how to support developers. For instance, the IDE should, as far as possible, help developers track down locations where significant changes occur as a result of a given change (say, a git commit). You could get a list of locations where there a non-trivial downcast has been introduced. Same thing when you run pub upgrade. But that's still about implicit downcasts in general, not about FunctionType-2-CallableObject downcasts.

By the way, changes may also introduce bugs around explicit casts (which includes C#, Java, C++ and others): If you claim to know that a given expression has a specific type, and somebody changes the code that you depend on such that this assumption is no longer justified, you can have a clean compile and still get a (newly introduced) run-time error at such a cast.

So we start with code that looks like this (hugely simplified from a real code base, of course):

The code you show is basically a scenario where (1) a type Callback is redefined to denote a proper subtype of its former value, and then (2) client code passing a value of the old type used to be a trivial cast and it is now an implicit downcast. No warning at compile time, but failure at run time. In that sense it's the same as this scenario:

class C { foo(num n) {}}
main() { num n = 3.14; new C().foo(n); }

.. where somebody later on commits the change to make it class C { foo(int n) {}}: suddenly the invocation in main has an implicit downcast, and it will fail at run time.

Granted, there are some extra elements in your example. In particular, the argument (x) => x in your example contains implicit type annotations which are subject to inference (making it an int Function(int) in both the "before" and the "after" scenario). The scenario could, for instance, give rise to a situation where the type inference "repairs" the code upon the change (say, because typedef int Callback(int i); is changed to typedef String Callback(String s);). However, this "self-repair" effect only occurs in special situations, and rather arbitrary (say, (n) => n.floor(); could have types int Function(num) and Floor Function(Room), and maybe a few other very ad-hoc types).

The essence of the example is still "Type T denotes S, then change it to denote a proper subtype U of S, and now clients passing values to parameters/variables typed T may have an implicit downcast that they didn't have before".

And we get no static feedback that our code is now broken.

You could actually get that in this particular case: You've introduced a notion of exact types (we know that new A() is an "exactly A" when that newExpression invokes a generative constructor, so we can detect the dynamic error statically at a downcast like B b = new A();), and we have something very similar here:

It can be detected statically that the type of (x) => x cannot ever be a callable class, no matter which type annotations type inference will add to it (including the return type which has no syntax, of course), so when Callback is a callable class we can statically detect the run time failure at Callback c = (x) => x;.

If you're really worried about this kind of situation then I'd suggest introducing this new kind of exactness: "The run-time type of this value is guaranteed to be a function type".

We didn't have a good reason to expect the implicit cast would succeed because we had no
idea an implicit cast was happening. They are implicit. That's the whole point.

That's still concerned with implicit cast in general and not so relevant for the FunctionType-2-CallableObject downcast in particular. The same applies to the remaining 5 paragraphs.

[Choose] recent CLs, find all of the implicit casts added by that CL, and ask the author of
the CL whether they intended/were aware of, the implicit cast that was added by their code

That would be really cool!

[leafpetersen]

I have no idea why you would presume this, and I think it reflects a fundamental misunderstanding of the way that most code comes into being.

On re-reading, this comment was more strongly worded and combative than it needed to be - my apologies.

[eernstg]

@leafpetersen, on re-reading, I can see why you thought that I was talking about downcasts in general, not just FunctionType-2-CallableObject -- counter-apologies. ;-)

By the way, you mentioned a different issue that might actually play a bigger role for this issue than symmetry and assignability: Type complexity.

It would be a problem if we support a type universe whose complexity is significantly greater than we intend. It could affect the comprehensibility of the language, the ability to maintain soundness, or the ability to obtain the type of expressions (in a reasonable amount of time, or at all). So if the subtype relationship between function types and callable classes creates this kind of issue, we should discuss that explicitly and address it directly.

An example of a situation that creates a peculiar kind of recursion is the following:

typedef T F<T>(T t);

class C<T extends F<T>> {}

class D extends C<D> {
  D call(D x) => x;
}

Class D is correct because D <: F<D>, that is D <: D Function(D), because D has a call method with type D Function(D). So entities of type C<...> can exist, for example C<D>.

But considering a raw C, instantiate-to-bound produces an infinite type of a kind that I haven't seen elsewhere (I think): C means C<W> where W = W Function(W), which may then be unfolded any number of levels, e.g., C<W Function(W Function(W...) ...) Function(W Function(W...) ...) ...>.

Is this what you were getting at?

If we aim to eliminate such a class of types that we deem "too complex", we might eliminate the subtype relationship between function types and callable classes entirely (which would of course eliminate all issues concerned with that subtype relationship, including assignability). In that case we would essentially get rid of callable classes entirely, because there's nothing special about call if everything you can do with it is to tear it off (even if that would happen implicitly when an instance of a "now-not-so-callable-class" is provided where a function is expected).

We might also be able to eliminate the overly complex types by adding some (hopefully simple and practical) constraints on the allowed declarations of callable classes.

[lrhn]

So, the problem is that we:

• allow nominative types to be recursive (function typedefs are not allowed to refer to themselves)
• allow function types as bounds in F-bounded classes.
• allow the type of the object with a call method to be reduced to its function type, throwing away the nominative indirection.

We can create infinite class type constraints with F-bounded classes, like class Comparable<T extends Comparable<T>>. That is "not a problem" except when we try to invent a solution, so we give up on that.

The problem here is that assigning a function typed value to a callable class tp means checking against a structural type against an F-bounded nominative type where the bound, so we do need to unfold arbitrarily far to check that the assignment is valid. Because of the covariance, we can switch the burden from side to side and force an infinite unfolding.

Assignment in the other direction is safe - it's an up-cast, not a down-cast, so we don't need to do runtime checking.

So, if we do implicit tear-offs instead of assigning the object itself when assigning a callable object to a function type, then we can certainly, and statically, reject assignments from function types to objects, which avoids the potentially infinite runtime type-check. I can see why it's desirable.

[eernstg]

I've created a separate dart-lang-evolution issue (152) to discuss the type system complexity issue.

[leafpetersen]

The problem here is that assigning a function typed value to a callable class tp means checking against a structural type against an F-bounded nominative type where the bound,

I don't think this has anything to do with F-bounded types, at least not uniquely. The original example from here:

#29791

is just:

class Foo {
  void call(void callback(Foo foo)) {}
}

main() {
  Foo foo = new Foo();
  foo(foo);
}

The question is, does Foo subtype Foo -> void.

• To answer that question, we need to know whether (Foo -> void) -> void subtypes Foo -> void
• To answer that, we need to know whether Foo subtypes Foo -> void, and we're back to where we started.

It's possible (likely)? That the algorithms developed to handle equi-recursive types (e.g. by Amadio and Cardelli) would handle this just fine, but that's quite a bit of machinery for a small feature. That is another direction we can explore though.

[jmesserly]

@leafpetersen is there anything worth fixing here at this point?

[lrhn]

I believe this is "fixed" by not making callable class's types subtypes of function types.

We then have an asymmetry in the other direction, since a Foo is assignable to void Function(Foo), but not vice-versa (there is no type relation between the two, but an automatic coercion in one direction).

Nothing more is expected to happen here.

Is it still an issue that the relation is not symmetric?

[jmesserly]

https://dart-review.googlesource.com/c/sdk/+/62660

[srawlins]

I'm closing this as stale. If there is still a symmetry issue in analyzer or the spec, feel free to re-open or file a new issue.