Analyzer issues error 'invalid override' in case of covariant argument and inherited implementation

[alexmarkov]

Test:

class A {}
class B extends A {}

class Base {
  void f(B arg) {}
}

abstract class Interface {
  void f(covariant A arg);
}

class C extends Base implements Interface {}

Analyzer output:

Analyzing foo.dart...
  error • Base class introduces an invalid override. The type of 'Base.f' ('(B) → void') isn't a subtype of 'Interface.f' ('(A) → void') at foo.dart:13:9 • strong_mode_invalid_method_override_from_base
1 error found.

The following similar test does not produce an error:

class A {}
class B extends A {}

class Base implements Interface {
  void f(B arg) {}
}

abstract class Interface {
  void f(covariant A arg);
}

class C extends Base {}

CFE doesn't flag any errors in both cases.

[bwilkerson]

@leafpetersen @lrhn I'm not sure what the right behavior is here. The spec still says "The modifier covariant is used in strong mode." and has no effect, but I'm guessing that's out of date :-)

[leafpetersen]

cc @eernstg @munificent @jmesserly @stereotype441

The feature spec here is phrased in terms of overriding methods and doesn't specifically address the case where there is no overriding method (just an inherited method). Implicitly then, I read the spec as saying that this is not allowed. @munificent @eernstg do you agree with my reading, and do you recall whether this was an intentional choice in the spec as written or just accidental?

[jmesserly]

IIRC, covariant predated the implementation of the runtime soundness checks for covariant generic classes. At the time we didn't have the mechanisms in place to generate the checking stub (if needed). So we might've made this an error to preserve soundness, or to keep the feature simpler? Not sure. But generic classes do support a similar example.

[lrhn]

If possible, I'd prefer to work the same way as the corresponding generic case.
Maybe that would be:

class A {}
class B extends A {}
class Base  {
  void f(B arg) {}
}
abstract class Interface<T> {
  void f(T arg);
}

class C extends Base implements Interface<A> {}

We don't currently accept that.

On the other hand, if we change C to:

class C extends Base implements Interface<B> {}

we do accept it. That would be the same as the example above where the interface adds covariant B as parameter type instead of covariant A.

[jmesserly]

we do accept it. That would be the same as the example above where the interface adds covariant B as parameter type instead of covariant A.

Great point! So the current behavior does look consistent. This example has no errors:

class A {}
class B extends A {}

class Base {
  void f(B arg) {}
}

abstract class Interface {
  void f(covariant B arg);
}

class C extends Base implements Interface {}

It looks like CFE doesn't produce an error for the original example, or for @lrhn's generic example above. Should we file a CFE bug for that?

[eernstg]

We do treat parameter types differently when they are covariant-from-class and when they are covariant-by-modifier.

In particular, a parameter which is covariant-by-modifier may be tightened arbitrarily, but a parameter which is covariant-from-class may not:

abstract class A1 { foo(covariant num n); }
abstract class A2 implements A1 { foo(int i); } // OK.

abstract class B1<X> { foo(X x); }
abstract class B2 implements B1<num> { foo(int i); } // Error.

The point is that we wish to maintain the guarantee that the actual type annotation on a parameter which is covariant-from-class follows the type variable of the enclosing class that occurs in its type (which is the initial mover that made the parameter covariant-from-class in the first place, so we always have such a type variable). If we want to allow B2 to tighten the parameter type not just to num (following the value of the type variable B1.X as passed in the implements clause of B2), but all the way to int, then we have to add an explicit modifier covariant on that parameter, somewhere.

So that's the reason why we can say Interface<B> here, but we cannot say Interface<A>, because then we have an implementation of foo, but its parameter type is tightened beyond the relevant type (C should have a void f(A) where A is covariant-from-class, so void f(B) won't do).

@jmesserly's example here includes the covariant modifier, but it also takes away the property that the parameter of f is covariant at all. The following variant adds covariant-by-modifier to the parameter and preserves the property that it is covariant-by-class, and that shows how we can get a permission to tighten a parameter which is covariant-from-class (because it can be covariant in both ways):

class A {}
class B extends A {}

class Base {
  void f(B arg) {}
}

abstract class Interface<X> {
  void f(covariant X arg);
}

class C extends Base implements Interface<A> {} // OK!

[Edit eernstg, April 2020: The following reference to "'Test:' here" is wrong, that example does have an error, because the parameter that causes the need for a forwarding stub is covariant-by-declaration.]

So the analyzer should stop emitting an error for the first example ('Test:' here), and code generation may have to generate an implicit forwarding method in C such that we can check the dynamic type of the actual argument (because Base.f has a parameter which is not covariant, and hence it will probably not contain such a type check). The next example shouldn't and doesn't cause any errors, which is correct.

The first example here is flagged as an error, which is correct. With Interface<B> it is not an error, also OK.

Finally, the example that I wrote above should be accepted, but the analyzer emits an error. In that case the implementation of Base.f is again unlikely to check the type of the actual argument, so we will again need a generated forwarder C.f which enforces that it is a B.