Skip to content

Value types are not properly compared in TypeComparer #2430

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
felixmulder opened this issue May 15, 2017 · 4 comments
Closed

Value types are not properly compared in TypeComparer #2430

felixmulder opened this issue May 15, 2017 · 4 comments

Comments

@felixmulder
Copy link
Contributor

felixmulder commented May 15, 2017

The following subtyping test fails Ycheck after erasure:

class Test1(val x: Int) extends AnyVal
class Test2(val y: Test1) extends AnyVal

In tree checker:

tree.tpe <:< pt // tree.tpe = ErasedValueType(Test2, ErasedValueType(Test1, Int))($this)
                //       pt = Int

This is what causes erasure not to be Ycheckable. The next phase, ElimErasedValueType, removes the ErasedValueTypes and casts so that it is again, Ycheckable.

ping @odersky

@felixmulder
Copy link
Contributor Author

After this issue is resolved, the work from #2415 makes erasure Ycheckable

@smarter
Copy link
Member

smarter commented May 15, 2017

ErasedValueType(A, B) is intentionally not a subtype or supertype of either A or B to trigger boxing/unboxing in Erasure, but Erasure should add the necessary casts (evt2u to convert an ErasedValueType to its underlying value: https://github.com/lampepfl/dotty/blob/master/compiler/src/dotty/tools/dotc/transform/Erasure.scala#L251). If after Erasure you have a tree with type ErasedValueType whose expected type is Int, then something is wrong, because the cast should have been inserted.

@odersky
Copy link
Contributor

odersky commented May 16, 2017

I agree with @smarter. This is intentional, and done so that erasure is forced to insert the proper casts.

@smarter
Copy link
Member

smarter commented May 16, 2017

I talked with @felixmulder and we did find one issue: when a value class underlying type is also a value class, you may need to wrap/unwrap using evt2u/u2evt multiple times, but this is apparently not always done, I haven't investigated more but @felixmulder is on it.

smarter added a commit to dotty-staging/dotty that referenced this issue Jul 27, 2017
During Erasure, A value class A with underlying type U has its type
semi-erased to ErasedValueType(A, [semi-erasure of U]). To make Erasure
typecheck, special cast functions A.evt2u$ and A.u2evt$ are used to cast
between the underlying type and the class EVT type. Before this commit,
this conversion was not always correctly done for nested value classes,
this did not lead to problem in practice but broke -Ycheck:erasure.
smarter added a commit to dotty-staging/dotty that referenced this issue Jul 27, 2017
During Erasure, a value class A with underlying type U has its type
semi-erased to ErasedValueType(A, [semi-erasure of U]). To make Erasure
typecheck, special cast functions A.evt2u$ and A.u2evt$ are used to cast
between the underlying type and the class EVT type. Before this commit,
this conversion was not always correctly done for nested value classes,
this did not lead to problem in practice but broke -Ycheck:erasure.
DarkDimius added a commit that referenced this issue Jul 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants