Breaking Change Request: Enforce covariance of type variables in super-interfaces

[leafpetersen]

It is currently possible to subvert the Dart type system in the class hierarchy because of a missing check on the use of covariant generic type parameters. Discussion of the consequences of this can be found here, but in short it can cause soundness violations. We propose to make the problematic uses errors as described here.

Change Summary After this change, using the type parameter of a generic class non-covariantly in a direct super-interface of a class would be an error. Example:

class A<X> {};
class B<X> extends A<void Function(X)> {};

The definition of B would cause a static error after this change.

Justification Missing this check allows Dart programs to subvert the type system, resulting in undefined (unsafe) behavior. For an example, see this issue.

Impact We expect the impact of this to be minimal. I have analyzed all internal google code and all of the flutter framework code and found no examples of code which would be impacted by this.

Mitigation We do not expect to be able to provide tooling to migrate any code impacted by this change. We might consider releasing this as a warning for some number of dev releases (or possibly even a stable release cycle) before promoting it to an error to ensure that users have time to migrate any affected code.

[matanlurey]

👍, thanks!

[mit-mit]

@Hixie LGTM'ed via email

[leafpetersen]

@mit-mit LGTM'ed via email

[mit-mit]

@leafpetersen what is the timeline for landing this breaking change?

[leafpetersen]

Implementation issue is here: dart-lang/language#113 . It's on the books for Q1, I tentatively marked it with the 2.2 milestone.

[vsmenon]

Is this still Q1?

[kmillikin]

What is the relative priority of this vs. UI as code? This is a small amount of work for CFE but we have only a few weeks left in Q1.

[mit-mit]

UI as code is more urgent

[mit-mit]

cc @leafpetersen remind me, did we send a breaking change notification for this to announce?

[leafpetersen]

Yes, I sent out announcements to the usual places.

[vsmenon]

@kmillikin @stereotype441 - Leaf added the impl items to 2.3 as P2. They are soundness issues, but probably rare. Nice to get done, but lower priority than ui-as-code.

[leafpetersen]

It looks like this is going to slip forward another release.

[dgrove]

LGTM

[aadilmaan]

Leaf - We have all the approvals. You may proceed with this in Dart2.4

[leafpetersen]

There should be no bad builds to be marked for flutter from the period in which this had landed in the CFE and not the analyzer. The last roll of the engine that I see is flutter/flutter@2f6a986, which includes commits 0308a1c4a Roll src/third_party/dart fde6a5917e..e3edfd36b2 (14 commits) . That commit range does not include either implementation commit.

[dgrove]

What's remaining here?

[aadilmaan]

Leaf will do the necessary announce via emails groups once we have a release candidate for D24 but will close this issue for now as no engineering work remains.