Skip to content

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

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

Open
2 of 4 tasks
eernstg opened this issue Apr 5, 2018 · 6 comments
Open
2 of 4 tasks
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@eernstg
Copy link
Member

eernstg commented Apr 5, 2018

[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
Copy link
Member

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

@eernstg
Copy link
Member Author

eernstg commented Apr 6, 2018

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.

@munificent munificent added area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). and removed area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). area-multi labels Jun 22, 2018
@a-siva a-siva added the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label Jul 9, 2018
@munificent munificent removed the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label Jul 11, 2018
@eernstg
Copy link
Member Author

eernstg commented Sep 12, 2018

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
Copy link

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
Copy link
Member Author

eernstg commented Sep 12, 2018

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!

dart-bot pushed a commit that referenced this issue Sep 14, 2018
The point is that #32782 requests a fix such that
`typedef F1 = void Function(int)` and `typedef void F2(int)`
satisfy that `F1 == F2` evaluates to true. This is marked
'p3-low' for the common front end but 'p1-high' for DDC.

It seems likely to me that this could create the situation where
code is developed using DDC, is working, and then fails upon
deployment using the vm. Also, there is a single case where
`dart2js` fails in the associated `type_alias_equality_test.dart`,
so deployment on the web would also fail upon deployment, though
only in some of the cases.

However, with some input from Aske I concluded that the situation
might have arisen because there _is_ no work to do for this in the
common front end, because it will be handled by the backend (which
also explains why `dart2js` has it almost right).

This CL is just introducing a tiny change: It changes the issue
indicated for all VM related failures in said test to point to
#32783, which is presumably the right issue for backend work.

Apart from that, I've added you, the reviewers, in order to make
sure that the relevant people get this heads up. We may then decide
to land this CL, change the priority on #31359, do nothing, or
whatever turns out to be the right response. ;-)

Change-Id: I92672547d7fe795e877604c0da1e0e4579e4e04a
Reviewed-on: https://dart-review.googlesource.com/74403
Reviewed-by: Jenny Messerly <[email protected]>
@eernstg
Copy link
Member Author

eernstg commented Oct 10, 2018

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

6 participants