Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Inconsistent errors for covariant subtyping violations on generic function fields. #36800

Closed
Markzipan opened this issue Apr 29, 2019 · 4 comments
Labels
closed-as-intended Closed as the reported issue is expected behavior

Comments

@Markzipan
Copy link
Contributor

For the following snippet:

typedef G<T> = Function<S extends T>(S s);

class C<T> {
  G<T> g;
}

main() {
  C<Object> c = new C<num>()
    ..g = <S extends num>(S s) => s.isNegative;

  c.g<String>('hi');
}

DDC emits a runtime type error:

Error: Expected a value of type '<S>(S) => dynamic', but got one of type '<S extends num>(S) => bool'

The VM emits a runtime method-not-found error:

Unhandled exception:                                    
NoSuchMethodError: Class 'String' has no instance getter
'isNegative'.                                           
Receiver: "hi"                                          
Tried calling: isNegative                               

Dart2JS emits a weirder runtime type error:

TypeError: Closure 'main_closure': type 'Closure' is not a subtype of type '(unexpected-generic-index:0) => dynamic'

DDK silently fails.

What's the expected behavior here? I would expect a runtime type assertion to report something along the lines of "String does not extend num".

@Markzipan Markzipan added the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label Apr 29, 2019
@lrhn
Copy link
Member

lrhn commented Apr 30, 2019

DDC seems correct, even if it's not that clear from the error message. It expected c.g to have type Function<S extends Object>(S) (because c has static type C<Object>). It doesn't print the extends Object because it is equivalent to no extends clause.
The actual value had type Function<S extends num>(S) which is not assignable to Function<S extends Object>(S), so you get a run-time error because the expression c.g does not have run-time type that satisfies its static type.

You can get into this situation because Dart has covariant class generics, and that's not type-safe. You hit one of the cases where that type-safety can occur, and where we insert extra run-time checks to catch it.

The rest are wrong.

I'm marking this as a front-end issue because the front end should make the implicit check explicit, and then the VM and dart2js would not let the invalidly typed value proceed.

@lrhn lrhn added area-front-end Use area-front-end for front end / CFE / kernel format related issues. front-end-missing-error and removed area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). labels Apr 30, 2019
@eernstg
Copy link
Member

eernstg commented May 1, 2019

Cf. dart-lang/language#296: The field g has non-covariant type (because it is not true that the type of g for a receiver of type S is a subtype of the type of g for a receiver of type T for all S <: T <: C<U> for some U).

The run-time type error from DDC arises because of a caller-side check on the value of c.g (and it fails to have the type <S>(S) => dynamic which is the type that we'd naively expect if we consider a receiver with static type C<Object>).

My proposal in dart-lang/language#297 is to use a sound type in the first place, which would give c.g the type Function.

That type is not very informative (it's "the type dynamic for functions", you could say), but there is no type which is more specific than that which is also a common supertype of all the possible types of c.g when c has a dynamic type which is a subtype of C<Object>. So we cannot promise more than that; the lesson for developers would be to avoid using silly types like G<T> for an instance variable, and in general, to use members whose type is non-covariant in the type variables of the class. ;-)

But right now the most consistent fix is probably to make sure that the backends get the same caller-side checks that DDC already has.

@eernstg eernstg added closed-as-intended Closed as the reported issue is expected behavior and removed area-front-end Use area-front-end for front end / CFE / kernel format related issues. front-end-missing-error labels Mar 6, 2023
@eernstg
Copy link
Member

eernstg commented Mar 6, 2023

I will close this issue now. Here is an updated version of the example:

typedef G<X> = Function<Y extends X>(Y); // `X` is invariant.

class C<X> {
  G<X> g;
  C(this.g);
}

main() {
  C<Object> c = C<num>(<Y extends num>(Y y) => y.isNegative); // Covariance is allowed, as always.
  c.g<String>('hi'); // The type of `c.g` is checked at run time.
  // The previous line throws because `c.g` has static type `G<Object>` and dynamic
  // type `G<num>`, and the latter is _not_ a subtype of the former.
}

The situation where an instance variable (g in the example) has a declared type (G<X>) where a type variable (X) declared by the enclosing class occurs in a non-covariant position is handled by implicitly generating code to perform a dynamic type check (also known as a caller-side check).

This means that there will not be any compile-time errors, but there will be a dynamic error at c.g<String>('Hi') (after evaluating c.g, we don't even get far enough to attempt to pass the type argument). This is also the implemented behavior.

It is still true that it is highly recommended to avoid having an instance variable whose type is non-covariant in the enclosing class type parameters.

@eernstg
Copy link
Member

eernstg commented Mar 6, 2023

Turns out that we did not have a linter request to avoid having declarations like C.g in the first place. Created #59050 in order to request a lint of this nature.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
closed-as-intended Closed as the reported issue is expected behavior
Projects
None yet
Development

No branches or pull requests

3 participants