Skip to content

strong_mode_invalid_method_override gets confused with complex type parameters #27546

New issue

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

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

Already on GitHub? Sign in to your account

Closed
abarth opened this issue Oct 7, 2016 · 8 comments
Closed
Labels
legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@abarth
Copy link
Contributor

abarth commented Oct 7, 2016

class X {}

class A<U extends X> {}

class B {
  A foo() => null;
}

class C<T extends A> extends B {
  @override
  T foo() => null;
}

This triggers a strong_mode_invalid_method_override on the override of foo, but that seems spurious. If you remove the <U extends X>, then the error goes away.

@abarth
Copy link
Contributor Author

abarth commented Oct 7, 2016

@leafpetersen

@bwilkerson bwilkerson added legacy-area-analyzer Use area-devexp instead. analyzer-strong-mode P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Oct 7, 2016
@abarth
Copy link
Contributor Author

abarth commented Oct 7, 2016

Possibly related test. This code does not appear to trigger any analyzer errors:

import 'package:test/test.dart';

class X {}

class Y extends X {}

class A<U extends X> {
  U u;
}

void main() {
  test('Asignment through a covariant template throws exception', () {
    A<Y> ay = new A<Y>();
    A<X> ayAsAx = ay;
    expect(() {
      ayAsAx.u = new X();
    }, throws);
  });
}

/cc @Hixie

@leafpetersen
Copy link
Member

This actually has nothing directly to do with overrides. The way that we treat uses of generic types where you leave off the arguments (I call these "raw types") has a few warts right now, and a notable one is around uses of raw types as bounds for generic type parameters.

The current treatment of <T extends A> in your example is to treat it as if you had written <T extends A<dynamic>>. This should give an error, since dynamic doesn't satisfy the constraint that A specifies for its parameter, but apparently we're not issuing that error. There is an open bug for fixing these warts here: #27526 .

In the meantime, you can fix your issue by specifying an argument for A in the bound instead of using a raw type, as such:

class C<T extends A<X>> extends B {
  @override
  T foo() => null;
}

Closing as a duplicate of #27526.

@leafpetersen
Copy link
Member

Just saw your second example, but not sure I understand it: are you expecting an analyzer warning there, and if so where and why?

@abarth
Copy link
Contributor Author

abarth commented Oct 7, 2016

Consider this line:

ayAsAx.u = new X();

On this line, the field u has type X and it is receiving a value of type X. If the analyzer doesn't complain about this program, that implies that this assignment requires a runtime check (i.e., to generate the exception we observe in the test). Do all assignments, even those with matching types on the left- and right-hand sides, require a check? I was under the impression that strong mode proved enough about the validity of the type annotations to let us skip such runtime checks (e.g., where the type annotations on the left- and right-hand side of an assignment match).

In other type systems, the type system would fail to prove that A<X> ayAsAx = ay; is well-typed because (those systems) generic types are neither covariant nor contravariant in their type parameter. That usually means that A<X> and A<Y> are incomparable types. However, strong mode seems to judge that line as well-typed, which means it needs a runtime check on ayAsAx.u = new X(); in order to maintain type safety.

@abarth
Copy link
Contributor Author

abarth commented Oct 7, 2016

Put a different way, given that a program is well-typed according to strong mode, when does an assignment (or a function call) require a runtime type check on the objects it receives (either as the right-hand side of the assignment or the actual value of a parameter)?

@leafpetersen
Copy link
Member

Right. So, first of all, you are running on the VM, which doesn't yet implement strong mode runtime checks. But it does implement checked mode, and it is the checked mode check that gives you the exception there. If you run that in production mode, you will get no exception.

Now to strong mode. Covariant generics are not sound in general. In order to support them, strong mode requires a check everywhere a type variable is used contra-variantly. You can think of this as a subset of the checked mode checks. These checks guarantee that values that actually reach a location have the type expected by that location, so that subsequent uses of the location can assume that it has the right type.

Note that these checks are only required for contra-variant uses of generic type parameters. So to your question:

Do all assignments, even those with matching types on the left- and right-hand sides, require a check?

class A {
  int x;
  int read() => 3;
  void write(int x) {}
  void callback(void f(int x)) {}
}

class B<T> {
  T x;
  T read() => null;
  void write(T x) {}
  void callback(void f(T x)) {}
}

void main() {
  A a = new A();
  a.x = 3; // no check
  int x = a.x; // no check
  num y = x; // no check
  x = y; // downcast, check

  x = a.read(); // no check
  a.write(x); // no check
  a.callback((int x) => x); // no check,

  B<int> b = new B<int>();
  b.x = 3; // covariance check (optimizable)
  x = b.x; // no check

  x = b.read(); // no check
  b.write(x); // covariance check (optimizable)
  b.callback((int x) => x); // no check, contra-variant

}

Note that the implied covariance check on the assignment to b.x can be optimized away in many cases. In this specific case, the fact that int has no subclasses makes it pretty trivial to eliminate the covariance check, but in general whenever the compiler can prove that a variable contains exactly the declared type (rather than some subtype) it should be able to eliminate the check.

Put a different way, given that a program is well-typed according to strong mode, when does an assignment (or a function call) require a runtime type check on the objects it receives (either as the right-hand side of the assignment or the actual value of a parameter)?

In general, whenever you assign to a field, or pass an argument to a parameter, where the type of the field/parameter uses the class type parameter in a contra-variant position (or is in an override of field/method that does so), there is an implied check (again, subject to optimization).

@Sfshaza maybe this should go in a FAQ somewhere?

@abarth
Copy link
Contributor Author

abarth commented Oct 7, 2016

Thanks that makes good sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

3 participants