-
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
Handle NoType in TypeComparer.disjointnessBoundary #21520
base: main
Are you sure you want to change the base?
Conversation
@@ -3057,6 +3057,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling | |||
disjointnessBoundary(tp.effectiveBounds.hi) | |||
case tp: ErrorType => | |||
defn.AnyType | |||
case tp: NoType.type => | |||
defn.AnyType |
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.
defn.NothingType
is another alternative, but using AnyType
gives error messages similar to what we get from Scala 3.3.
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.
defn.AnyType
is definitely better. NoType
shouldn't be here. At best we have to treat it like ErrorType
just above.
Perhaps add an assertion that we only get here under -source 3.3
? Like
assert(!sourceVersion.isAtLeast(SourceVersion.`3.4`),
"Unexpected NoType found in disjointnessBoundary")
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.
I believe the example I added to #20265
trait Expr[T]
trait Ops[T](using m: scala.deriving.Mirror.ProductOf[T]) {
def apply(args: Tuple.Map[m.MirroredElemTypes, Expr]): Expr[T] = ???
}
case class P(a: Int)
object P extends Ops[P]
hits this crash without specifying -source 3.3
.
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.
Then it should also be added as a test.
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.
@sjrd I've added it as tests/neg/i20265-1.scala
I found an added another piece of code hitting this crash in the issue #20265 Maybe it worth revisiting this fix? It also seems to fix my issue. |
I see @sjrd is the assignee, maybe he has some thoughts on this? Not sure how to bring this further. |
Closes #20265
Note that the example in that issue already does not compile on 3.5.0 (and 3.4.3), failing with a reasonable error message. However, opting in to ignoring the issue with
-source:3.3
does cause a crash.