In front end, do not relax override rules when overridden parameter is covariant

[stereotype441]

Consider the following code:

class A {}
class B extends A {}
class C {
  void f(B x, B y) {}
}
abstract class I1 {
  void f(covariant A x, B y);
}
class D extends C implements I1 {}

As pointed out in #31580 (comment), the class D should be rejected at compile time because its implementation of f has type void Function(B, B), which is not a valid override type for the member type of f in the interface of D, void Function(covariant A, B).

I have not yet implemented the override checking rules in the front end (I will soon). This bug serves as a reminder that the case above should result in an error.

[eernstg]

Based on an IRL discussion with Lasse: Considering this issue again, and again, it seems less disruptive, and less confusing, if we do allow the compilers to generate forwarders (and hence we consider D to be a correct declaration). We should then document this mechanism in the specification(s), and then we can tell developers that we know what we're doing, and it's intentional, if they ever report that "WOT?!" experience that I mentioned in #31580 (comment).

The argument that actually killed the idea of requiring explicit forwarders for me was that a plain forwarding declaration, foo(int x) => super.foo(x); doesn't work at all to document that that "Ah, of course, the argument x at this location is covariant, and the inherited one is not, so we declare this forwarder to explicitly state the commitment that x is covariant. And, obviously, we don't have to write covariant here, because it's implicitly covariant due to implements I<String>". It just doesn't have the intended documentational effect after all.

@leafpetersen, I suspect that it's OK with you to allow these forwarders to be generated implicitly; @lrhn, I believe that I heard you talk in favor of this approach earlier today?

I'm sorry about going back and forth on this topic, but I think we should make the right decision rather than just fixing one decision that seemed to be in favor at some point.

If so, it is no longer a 'front-end-missing-error'; it may be a no-op, but I don't know the implementation details well enough so that may be wrong.

[eernstg]

@leafpetersen, @lrhn: Do you agree that we should allow generating these forwarders after all (and get them specified, too, of course)?

[lrhn]

Reluctantly, yes.
The case:

class C {
  void foo(int x) { ... }
}
abstract class I<T> {
  void foo(T x);
}
class D extends C implements I<int> {
}

it just looks like it should work. It'll be very hard to explain to users why it doesn't.

[leafpetersen]

lgtm

[eernstg]

OK, cool! So, Paul @stereotype441, this is no more a "front-end-missing-error". Maybe this means that there is nothing to do, because dartdevc already generates these forwarders? I won't close the issue because I might be wrong, but I suspect that it can be closed.

[stereotype441]

@eernstg I'm not 100% sure what the implementations currently do. I'll create a language_2 test verifying the behavior that I believe we want, and I'll send it to you, @leafpetersen, and @lrhn for review. Then I'll decide what to do with this bug based on whether that test passes or not :)

[eernstg]

Cool!

[stereotype441]

Ok, here's what I've found:

• The analyzer currently rejects code like the example at the top of this bug.
• dartdevc also rejects it, since it is based on the analyzer.
• dart2js and the VM currently accepts the code above, but they only throw the appropriate TypeError when running in checked mode.

I've created a CL with a test demonstrating these behaviors: https://dart-review.googlesource.com/#/c/sdk/+/31750

All of these problems should theoretically take care of themselves when these tools are transitioned over to use the unified front end. I think the presence of the failing test should be a sufficient reminder to ensure that everything works properly once the various tools are transitioned over to the unified front end, so unless someone objects, I'll close this bug once the aforementioned CL lands.

[eernstg]

One thing we hadn't discussed so far was the source of the signature of the induced forwarding method. I've argued in the CL that the forwarder should get the signature from the forwardee (in the example it is C.f: void Function(B, B)), rather than the signature from the interface (void Function(A, B)).

Apart from the technical reasons for doing so (given in the CL comment), the conceptual justification could be that the forwarder is a "generated corrected version" of the forwardee C.f (which will perform the dynamic type checks on the actual arguments that the forwardee could not know we need), it's not a "generated implementation" of the method signature from the interface I1.f. That makes sense, too, because an "implementation of I1.f" could do anything, but it's reasonable to expect a "corrected version of C.f" to forward to C.f.

[leafpetersen]

@eernstg I'm a bit skeptical about using the less specific type for the interface of D.f there.

• On a theoretical level, all other things being equal I'd like to have the property that nominal subtypes are structural subtypes as well. Choosing the C.f interface breaks this. Obviously covariant overrides in general break this, but still, all other things being equal I lean away from deviating here.
• From a user intent perspective, my sense is that if someone writes extends C implements I1, they mean either "uses the implementation of C to implement I1" or "implements the combination of the interfaces of C and I1", but probably not "uses the implementation of C to implement the interface of C, ignoring the interface of I1". It's not quite as black and white as that presentation makes it sound - it's possible that the user is not really interested in the version of f from I1 and only gets it by happenstance, but this seems less likely than either of the first two interpretations.
• It's slightly odd that promoting to the super type would give you fewer errors. We already have that with covariant generics and covariant overrides in general, but I still feel like erring on the side of sanity where possible.

I guess the counter-argument is that the user gets more accurate static errors if we choose the C.f signature, since the actual implementation signature is in fact that of C.f. And I suppose another way of phrasing the "user intent" argument is that the user intends instances of D to be usable at I1, but may want D to have a more precise signature - that's kind of the point of covariant overrides.

@stereotype441 brings up a very good point from my perspective: if C is abstract, then presumably we would choose the I1.f signature? If not, on what basis do we not? And if so, then I find it quite odd that we would choose a different signature depending on whether or not C is abstract. I don't know if it's possible, but it seems like a good principle that choosing the interface of a class should be done based on super-interfaces, not based on implementations.

[eernstg]

nominal subtypes are structural subtypes as well

Let me see if I can say what this means, such that we can check that we understand it in a similar manner: A nominal subtype is introduced by a subtype related syntactic clause (extends, implements, with), and a structural subtype is introduced by composite type expressions S and T where a certain setup of subtyping relationships exist for subterms of S and of T allow us to conclude S <: T.

So with class B extends A {} we have a nominal subtype relation B <: A and a structural subtype relation like B Function(A) <: A Function(B). But the latter would be a structural subtype relationship based on a nominal one, and so is List<B> <: List<A>, and List<B> <: Iterable<A> involves two steps where one is nominal and the other is structural, so we probably can't keep the two entirely separate.

I'm not 100% sure how I could map that to the choice of typing new D().f as void Function(A, B) vs. typing it as void Function(B, B).

One interpretation that does come to mind is that, with D d; I1 i1;, the static type of d.f is a subtype of the static type of i1.f with the former choice, which is not true with the latter choice. So the latter choice will make someReceiver.f a "contravariant expression": Even when the type of someReceiver gets more special there is no guarantee that the type of someReceiver.f will get more special.

However, that's exactly what developers must expect when they see void f(covariant A x, B y); in I1, and we have, of course, adjusted the reified type of tear-offs such that someReceiver.f does yield a value whose type is a subtype of the statically known type.

So I can see the temptation to ascribe the type void Function(A, B) to D.f because it preserves the monotonic relationship (typeof(y) <: typeof(x) => typeof(y.f) <: typeof(x.f)). But the presence of covariant in I1 serves exactly to inform developers that "This member does not have that property!".

On the contrary, I think we have good reasons for ascribing the type void Function(B, B) to D.f, that is, to take the signature from the actual forwardee.

The most prominent one is simply the semantics of the construct that we're discussing. Here we have it, explicitly:

class A {}
class B extends A {}
class C {
  void f(B x, B y) {}
}
abstract class I1 {
  void f(covariant A x, B y);
}
class D extends C implements I1 {
  void f(B x, B y) { super.f(x, y); }
}

This example basically shows the code that we're generating. The core conceptual reason why I think we should take this into account when we ascribe a type to D.f is that the type of an expression should strive to describe the run-time value as accurately as possible (basically, that's a combination of (1) intuitive soundness: don't lie, and (2) accuracy: don't gratuitously forget information).

Another reason is that the type of the forwardee may well yield the solution to a computational task that we are otherwise rejecting to perform in a compiler/analyzer:

class A {}
class B extends A {}
class C {
  void f(B x, [B y]) {}
}
abstract class I1 {
  void f(covariant A x);
}
abstract class I2 {
  void f(A x, B y);
}
class D extends C implements I1, I2 {}

In this example, we couldn't use the signature from I1.f as the type of D.f because it is not a correct override for every method signature for f in the superinterfaces of D (and I2.f wouldn't do, either). But we can use the one from C.f.

If we can obtain a well-formed class by inheriting some method (forget covariance for a minute) then the signature of that method must be a correct override for all signatures of f.

So, taking covariance into account, if we need to implicitly generate a forwarder for such an (otherwise) inherited method, it will work to adopt the method signature from the forwardee (amended with covariance as needed, simply because the forwarder is placed in D), and it may not work to adopt the signature that contributes covariance for some parameters. On top of that, we may of course have multiple signatures contributing covariance to different parameters, which makes it even less workable to say "take the method signature that contributes the covariance".

From a user intent perspective..

I actually think that the method signature from the actual forwardee fits quite nicely for "uses the implementation of C to implement I1", just like the following:

class C {
  void f(Object o) {}
}
abstract class I1 {
  void f(int i);
}
class D extends C implements I1 {}

main() => new D().f('Hello, int!'); // Fine!

In this situation we also adopt the signature from the inherited implementation, even though it isn't identical to the signature in the class that D implements. So there's no way we could expect implements I1 to imply that each method that I1 has (declared or obtained by subtyping) must have a method signature which is identical to the one that I1 has. In particular there is no guarantee that this signature is the most specific, and when there is some other method signature which is strictly more specific (as in this case) then it obviously won't work.

Maybe the general phrase should be "uses the implementation of C to implement an interface which conforms to the interface of I1". It is then implied that it also implements the interface of I1, but not that they are identical, and that's a property that we can actually rely on.

if C is abstract, then presumably we would choose the I1.f signature?

If C is abstract and C.f is abstract then (in a roughly similar example, anyway) D is also abstract, and in that case we would presumably not need an implicitly generated forwarder. There must still be a most specific method signature for each method, but that's still true here:

class A {}
class B extends A {}
abstract class C {
  void f(B x, B y);
}
abstract class I1 {
  void f(covariant A x, B y);
}
abstract class D extends C implements I1 {}

In the interface of D, f has the signature void Function(A, B). If some subclass E of D is concrete it must have an implementation of f, and we may then reapply the rules we're discussing here on E.

If C is abstract and C.f is implemented I believe we have exactly the same situation as with a concrete C: The implementation of f from C possibly cannot be inherited directly (because it may not include dynamic type checks on the actual arguments as needed), but it should be "inherited" in a more loose sense, so we must insert an implicitly generated forwarder into E.

the interface of a class should be done based on super-interfaces, not based on implementations

I don't think we can make that distinction: We may obtain implementations from direct or indirect superclasses (including the ones obtained via mixins), but the interface of the superclass is based on the interfaces of its superclasses in turn, and the interface of the superclass is just one more superinterface, which contributes to the interface of the class in focus. So whenever we have an implementation it will contribute to the interface of the class just like all the non-implementation method signatures that we get from the implements hierarchy.

I think this means I wasn't really convinced. ;-)

[lrhn]

tl;dr - Let the synthetic forwarder be an implementation-only thing, with the signature of the implementation it forwards to + covariant, and keep it out of the interface so the D.foo signature is inherited normally from the super-interfaces

On Wed, Jan 3, 2018 at 7:43 PM, Leaf Petersen ***@***.***> wrote: @eernstg <https://github.com/eernstg> I'm a bit skeptical about using the less specific type for the interface of D.f there. - On a theoretical level, all other things being equal I'd like to have the property that nominal subtypes are structural subtypes as well. Choosing the C.f interface breaks this. It's not the C.f *interface*, it's the C.f *implementation*, with its

signature updated to be covariant. That might be a little too subtle, but I believe it's a good trade-off between rejecting programs that would function and not doing too much magic. I really, really hope it won't happen much in practice (because I hope covariant won't happen much, except as generics). The alternative, as I see it, is to say that a non-covariant implementation parameter cannot be considered a valid implementation of a covariant interface parameter unless the non-covariant implementation would be a valid implementation of a corresponding non-covariant interface parameter too.

- Obviously covariant overrides in general break this, but still, all other things being equal I lean away from deviating here. I would definitely not want to change the parameter types of the

*implementation*. Making the parameter covariant is stretching it already, and if it wasn't for my cases like example above, I'd probably have voted for just rejecting the program.

- From a user intent perspective, my sense is that if someone writes extends C implements I1, they mean either "uses the implementation of C to implement I1" or "implements the combination of the interfaces of C and I1", but probably not "uses the implementation of C to implement the interface of C, ignoring the interface of I1". ... but you *are* using the implementation of C, so that is what the

concrete implementation should be. I'm quite adamant that the type of the forwarder should be based on the implementation it forwards too. Now, we can discuss whether the synthetic forwarder should affect the *interface.* In the case of class D extends C implements I {} that would mean that ... - If the synthetic `void foo(covariant B, B)` forwarder method is considered as declared on D, then that will be the D interface signature of foo. - If we ignore the synthetic forwarder, then we have to pick the most specific of `void foo(B, B)` from C and `void foo(covariant A, B)` from I. That's the one from I. Which is an interesting point in any case: If you have two interfaces provide the same function, one with a covariant argument and the other being more specific, would we synthesize the resulting interface signature to be most-specific + covariant? We probably should (so type overrides and covariant overrides are orthogonal, and can come from different super-interfaces and both affect the resulting interface, we just won't combine types from different interfaces). Another example: class A { void foo(num x) {}; } class B extends A { void foo(covariant num x); // auto-forwarder here? Probably yes. (No interface issue here, but still needs to work). }

- It's not quite as black and white as that presentation makes it sound - it's possible that the user is not really interested in the version of f from I1 and only gets it by happenstance, but this seems less likely than either of the first two interpretations. - It's slightly odd that promoting to the super type would give you fewer errors. We already have that with covariant generics and covariant overrides in general, but I still feel like erring on the side of sanity where possible. I guess the counter-argument is that the user gets more accurate static errors if we choose the C.f signature, since the actual implementation signature is in fact that of C.f. And I suppose another way of phrasing the "user intent" argument is that the user intends instances of D to be usable at I1, but may want D to have a more precise signature - that's kind of the point of covariant overrides. @stereotype441 <https://github.com/stereotype441> brings up a very good point from my perspective: if C is abstract, then presumably we would choose the I1.f signature?

If the class is abstract, then we probably won't be adding any synthetic forwarders. In that case the interface won't include the synthetic forwarder (obviously). I think it's a good argument that the interface should not change even if we do add the forwarder, so I vote to keep the forwarder out of the interface computation. It's an *implementation* detail only (like abstract methods are interface details only and doesn't affect implementation).

If not, on what basis do we not? And if so, then I find it quite odd that we would choose a different signature depending on whether or not C is abstract. I don't know if it's possible, but it seems like a good principle that choosing the interface of a class should be done based on super-interfaces, not based on implementations.

The interface of a class should be inherited from super-interfaces (and if a method exists in more than one super-interface, pick the most specific one, and (new) inherit covariance from any superinterface) except where the class itself declares a member. If it does declare a member, that member's signature always win (but must be a valid override of all corresponding super-interface members). So, in this case, if we consider the synthetic forwarder as not declared by the class, the interface works, and the new implementation must still be valid for that interface. That means: class C { void foo(num x) {} } abstract class I { void foo(covariant String x); } class D extends C with I {} // COMPILE-TIME ERROR! We can't find a signature for D.foo from the super-interfaces, and even if we could, a forwarded like `void foo(covariant num x) => super.foo(x);` will not be a valid implementation of `void foo(covariant String)` from the I interface.