Specify that an implicit forwarder is generated for covariant-by-class, *and* for covariant-by-declaration

[eernstg]

[Edit Sep 2021: Said forwarders have actually been generated for a while by the CFE; the language team decided that it should then be allowed, so the CFE does not need to report the error. Changed the title to reflect this update. The implementation is handled in dart-lang/sdk#47072.]

Cf. this comment, the language team decided that it is a compile-time error to inherit a method implementation C.f that has a parameter p which is not covariant into a class D such that the member signature of f in the interface of D has the modifier covariant on p (which just means that p is covariant-by-declaration as seen from D).

But it is not an error when p is covariant-by-class as seen from D.

Here is an example of the first situation:

class A {}
class B extends A {}

class C {
  void f(B b) {}
}

abstract class I {
  void f(covariant A a);
}

class D extends C implements I {} // Error.

Here is an example of the second situation, involving covariance-by-class:

class A {}
class B extends A {}

class C {
  void f(B b) {}
}

abstract class I<X> {
  void f(X x);
}

class D extends C implements I<B> {}  // OK.

One way to see what is going on is to consider the implementation: In the first example, it is unsound for C.f to be used directly as the implementation of D.f. This is because D.f may be called with an actual argument whose static type is A, and it is up to the implementation of D.f to check that it is actually a B, but the declaration of C.f has no information to justify the addition of such a dynamic check to the body.

In the second example the same soundness problem arises, but because of the actual usage of code with this structure we decided that we would handle the soundness issue by generating a forwarding stub D.f which checks the dynamic type of the actual argument and then invokes C.f.

A question arises in the situation where a parameter is covariant-by-class as well as covariant-by-declaration:

class A {}
class B extends A {}

class C {
  void f(B b) {}
}

abstract class I<X> {
  void f(covariant X x);
}

class D extends C implements I<A> {}  // OK.

In this situation the fact that x is covariant-by-class in I justifies the generation of a forwarding stub, but note that the stub should check that the actual argument is B and then forward by calling super.f(x), such that the invocation of C.f is sound without further argument type checks.

This mechanism is not currently specified: The language specification tacitly implies that a forwarding stub is generated in all cases (because otherwise a soundness issue would arise, and there is no explicit language making any of the cases an error).

Additional related issues: dart-lang/sdk#31580, dart-lang/sdk#31596, dart-lang/sdk#34329, dart-lang/sdk#41371.

[leafpetersen]

Is this strictly a specification issue or is there implementation work implied?

[eernstg]

The topic is that it is in some cases unsound to inherit an implementation, and we have no specification text indicating that this is sometimes an error, and sometimes we require the implementations to generate a forwarding method that performs the check.

The current specification actually (but tacitly) implies that we would always generate a forwarding method if needed.

So we have a clear specification-or-implementation issue at hand: Either we need to say when it's an error, or we need to ask tool teams to generate those extra forwarding stubs. But we decided (here) to make it an error when the reason to need the dynamic check is that a parameter is covariant-by-declaration. So I believe that we can say that there definitely is a specification element to this issue.

For the implementation effort, we'd certainly look at existing tests. Considering the tests in tests/ language/covariant, we have the following failures:

FAILED: dart2analyzer-none release_x64 language/covariant/subtyping_with_mixin_test
-----------------
FAILED: none-vm release_x64 language/covariant/subtyping_test
FAILED: none-vm release_x64 language/covariant/subtyping_with_mixin_test

The test subtyping_with_mixin_test.dart was written with an expectation that contradicts the rule adopted here, so we'd need to adjust the test to expect a compile-time error at the declaration of C.

subtyping_with_mixin_test.dart is rejected by the analyzer with an error message that may indicate a different problem (it never says covariant), but the error does occur at the expected location. The program is rejected by the vm at run time (so this is currently a missing compile-time error in the front end, which is not bad, and there is an apparently totally unrelated bug that makes it fail at run time).

[eernstg]

Spec update in #1770. Tests https://dart-review.googlesource.com/c/sdk/+/208504. Discussion still ongoing.

[eernstg]

Closing: The language team decided that the error should be eliminated, and instead the correct behavior is to generate a forwarding stub such that the required dynamic checks are performed. Cf. #47072.