Skip to content

NNBD: Both dart and analyzer allow inherit two interfaces with incompatible type parameters: Object? vs dynamic and Object? vs void. #40453

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
iarkh opened this issue Feb 4, 2020 · 5 comments
Labels
closed-as-intended Closed as the reported issue is expected behavior

Comments

@iarkh
Copy link
Contributor

iarkh commented Feb 4, 2020

Dart VM version: 2.8.0-edge.40f23c735f04433e4fc334fbd674474bd3de0f8b (Tue Jan 28 01:14:48 2020 +0000) on "linux_x64"

The following source code declares classes X1 and X2 which inherit classes with incompatible type arguments:

class A<T>{}
class B implements A<Object?> {}

class C implements A<void> {}
class D implements A<dynamic> {}

class X1 extends B implements C {}
class X2 extends B implements D {}

main() {}

This code throws compile error with previous dart versions (for example, 2.7.0) and it seems like dart should do so, for example, see Classes defined in opted-in libraries in NNBD Spec:

If a class C in an opted-in library implements the same generic class I more than once as I0, .., In, and at least one of the Ii is not syntactically equal to the others, then it is an error if NNBD_TOP_MERGE(S0, ..., Sn) is not defined where Si is NORM(Ii). Otherwise, for the purposes of runtime subtyping checks, C is considered to implement the canonical interface given by NNBD_TOP_MERGE(S0, ..., Sn).

However, current both dart and analyzer do not throw error here, sample output is:

$ dart --enable-experiment=non-nullable test.dart
$ dartanalyzer --enable-experiment=non-nullable test.dart
Analyzing test.dart...
No issues found!

@lrhn lrhn changed the title NNBD: Both dart and analyzer allow inherit two interfaces with incompatible type parameters: Object? vs dynamic and Obhect? vs void. NNBD: Both dart and analyzer allow inherit two interfaces with incompatible type parameters: Object? vs dynamic and Object? vs void. Feb 4, 2020
@lrhn
Copy link
Member

lrhn commented Feb 4, 2020

I believe that NNBD_TOP_MERGE(A<Object?>, A<dynamic>) is well-defined.
We are still arguing whether it should be A<Object?> or A<dynamic>, the current specification says A<Object?>, but it will be one of them, and not an error.

@eernstg Agree?

@iarkh
Copy link
Contributor Author

iarkh commented Feb 4, 2020

OK, but anyway - Object? vs void is incorrect case and exception should be thrown?

@lrhn
Copy link
Member

lrhn commented Feb 4, 2020

NNBD_TOP_MERGE(A<Object?>, A<void>) is currently defined to give A<void>, and it's not an error either.

@iarkh
Copy link
Contributor Author

iarkh commented Feb 4, 2020

NNBD_TOP_MERGE(A<Object?>, A<void>) is currently defined to give A<void>, and it's not an error either.

OK, so seems like this bug should be closed as work-as-designed.

@a-siva a-siva added the closed-as-intended Closed as the reported issue is expected behavior label Feb 4, 2020
@a-siva a-siva closed this as completed Feb 4, 2020
@eernstg
Copy link
Member

eernstg commented Feb 5, 2020

@lrhn wrote:

Agree?

Yes. The approach based on normalization and then nnbd top merging is actually considerably more flexible than the old rules: We had 'It is a compile-time error if a class $C$ has two superinterfaces
that are different instantiations of the same generic class', and even though it was never spelled out exactly what "different" meant, it would surely not be easy to argue that List<FutureOr> and List<void> aren't different. But with the new rules they can coexist.

So we do allow more. In particular, the following assumption is too strong:

@iarkh wrote:

This code throws compile error with previous dart versions
(for example, 2.7.0) and it seems like dart should do so .. in NNBD

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

No branches or pull requests

4 participants