dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.22k stars 1.57k forks source link

`Type` values obtained from a typedef should represent the underlying type, not the typedef #32782

Open eernstg opened 6 years ago

eernstg commented 6 years ago

[Edit eernstg as indicated here: adding omitted x to F3.]

Cf. this and this comment, certain equalities should hold for objects of type Type. In particular, consider the following test:

typedef F1 = void Function(int);
typedef F2 = void Function(int);
typedef void F3(int x);

typedef G1 = X Function<X>(X);
typedef G2 = X Function<X>(X);
typedef G3 = Y Function<Y>(Y);

main() {
  Expect.equals(F1, F2); //# 01: ok
  Expect.equals(F1, F3); //# 02: ok
  Expect.equals(G1, G2); //# 03: ok
  Expect.equals(G1, G3); //# 04: ok
}

The specified equalities are expected because the given Type values should be compared based on the underlying type (such as void Function(int)) rather than the type alias as such (such as F1). This is in line with the fact that type aliases are aliases for existing types, rather than nominally distinct types:

F1 f = ...;
F2 g = f; // OK: `F1 <: F2` because it is the same type.
List<F2> xs = <F1>[]; // OK: `List<F1>` and `List<F2>` is the same type.

However, current Dart tools do not implement this comparison as expected above. This issue is concerned with adjusting the tools such that the test succeeds. It is allowed (and probably useful) for Type.toString(), stack traces, error messages, etc. to refer to the names of type aliases rather than (or in addition to) the underlying function type, this issue is only about the semantics of Type.operator ==.

Subtasks

Note that it is possible that the vm, dart2js, and dartdevc will automatically have this issue resolved when the common front end resolves it.

rakudrama commented 6 years ago

Is there a test that will pass once the subtask(s) are complete?

eernstg commented 6 years ago

Yes, type_alias_equality_test.dart is the file that contains the above example code, and status entries carry the issue numbers of the subtasks:

tests/language_2/language_2_dart2js.status:type_alias_equality_test/01: RuntimeError # Issue 32784
tests/language_2/language_2_dart2js.status:type_alias_equality_test/02: RuntimeError # Issue 32784
tests/language_2/language_2_dart2js.status:type_alias_equality_test/03: RuntimeError # Issue 32784
tests/language_2/language_2_dart2js.status:type_alias_equality_test/04: RuntimeError # Issue 32784
tests/language_2/language_2_dartdevc.status:type_alias_equality_test/01: RuntimeError # Issue 32785
tests/language_2/language_2_dartdevc.status:type_alias_equality_test/02: RuntimeError # Issue 32785
tests/language_2/language_2_dartdevc.status:type_alias_equality_test/03: RuntimeError # Issue 32785
tests/language_2/language_2_dartdevc.status:type_alias_equality_test/04: RuntimeError # Issue 32785
tests/language_2/language_2_dartdevc.status:type_alias_equality_test/02: RuntimeError # Issue 32785
tests/language_2/language_2_kernel.status:type_alias_equality_test/02: RuntimeError # Issue 31359
tests/language_2/language_2_kernel.status:type_alias_equality_test/03: RuntimeError # Issue 31359
tests/language_2/language_2_kernel.status:type_alias_equality_test/04: RuntimeError # Issue 31359
tests/language_2/language_2_kernel.status:type_alias_equality_test/02: RuntimeError # Issue 31359
tests/language_2/language_2_kernel.status:type_alias_equality_test/03: RuntimeError # Issue 31359
tests/language_2/language_2_kernel.status:type_alias_equality_test/04: RuntimeError # Issue 31359
tests/language_2/language_2_vm.status:type_alias_equality_test/01: RuntimeError # Issue 32783
tests/language_2/language_2_vm.status:type_alias_equality_test/02: RuntimeError # Issue 32783
tests/language_2/language_2_vm.status:type_alias_equality_test/03: RuntimeError # Issue 32783
tests/language_2/language_2_vm.status:type_alias_equality_test/04: RuntimeError # Issue 32783

Again, I hope that this can be fixed in the common front end and then the other tasks will just evaporate, but I couldn't know for sure so I created subtasks for the tools where I could see the issue.

eernstg commented 6 years ago

Note that the entire topic of Type instance equality (operator ==) is being clarified in #34447. However, the current issue has been decided in the language team, so the semantics requested in this issue is still valid (and the most likely outcome of #34447 is that other cases will use an approach which is a straightforward generalization of this issue).

kmillikin commented 6 years ago
typedef F1 = void Function(int);
typedef F2 = void Function(int);
typedef void F3(int);

typedef G1 = X Function<X>(X);
typedef G2 = X Function<X>(X);
typedef G3 = Y Function<Y>(Y);

main() {
  Expect.equals(F1, F2); //# 01: ok
  Expect.equals(F1, F3); //# 02: ok
  Expect.equals(G1, G2); //# 03: ok
  Expect.equals(G1, G3); //# 04: ok
}

I can't see how equals(F1, F3) is true. int is not reserved, so in F3 it's a parameter name with no type annotation and F3 is void Function(dynamic). Maybe I'm reading the grammar wrong?

eernstg commented 6 years ago

No, the test is buggy and I hadn't seen that (when I wrote it I forgot that int is the parameter name, not the type, and I'm sure I'll forget that also the next time I write one of those pesky old style function types). Thanks for catching it!

eernstg commented 6 years ago

The work on tests concerned with instantiate-to-bound and super-bounded types has a dynamic aspect, and for those tests it is important that we can compare Type instances from type aliases according to their underlying types.

@kmillikin, I can see that the subtask for fasta, the vm, and dart2js are still open, and I suspect that they will get this from the common front end (that is, simultaneously, when the front end has it). Do you agree, or is there a backend part as well? Also, is this issue in the pipeline, or could it take a while before it is expected to happen?