-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix #4323: enter class type parameters in GADT bounds #4351
Conversation
test performance please |
performance test scheduled: 1 job(s) in queue, 0 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/4351/ to see the changes. Benchmarks is based on merging with master (8feb596) |
case IExpr(value: Int) extends Expr[Int] | ||
case BExpr(value: Boolean) extends Expr[Boolean] | ||
|
||
def join(other: Expr[T]): Expr[T] = (this, other) match { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a neg test where this.T
is not other.T
:
def join[A](other: Expr[A]): Expr[T] = ...
It'd be good to figure out why a test had to be moved to pos-deep-subtype, this often masks real bugs. |
In fact the standard library had to be moved to -Yno-deep-subtypes, which seems even more concerning? We already know GADT bounds can create subtyping loops (#4069), so I'm not sure how robust this can possibly be. |
If this is the same kind of loop as #4069 then we should probably try to fix the underlying issue first (I can give it a go). |
For the test case
@smarter I think it is the same subtyping loop, please go ahead. |
Close for now, maybe @AleksanderBG can have a look at the GADTMap refactoring. |
Fix #4323: enter class type parameters in GADT bounds