Skip to content
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

Don't trust case class extractors with explicit type arguments #15669

Merged
merged 1 commit into from
Jul 14, 2022

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jul 13, 2022

#6551 introduced an exception where type tests of parameterized case classes were not flagged
as "type test cannot be checked at runtime". The idea was that the parameters would be inferred
by GADT reasoning and would therefore be correct.

But with #15356 we now also allow explicit type arguments in extractors. If these are given
we cannot guarantee that a type test for a case class in an unapply will always succeed.
So we need an unchecked warning.

Fixes #15662

scala#6551 introduced an exception where type tests of parameterized case classes were not flagged
as "type test cannot be checked at runtime". The idea was that the parameters would be inferred
by GADT reasoning and would therefore be correct.

But with scala#15356 we now also allow explicit type arguments in extractors. If these are given
we cannot guarantee that a type test for a case class in an unapply will always succeed.
So we need an unchecked warning.
@strelec
Copy link

strelec commented Jul 13, 2022

I disagree this being a compile time error.

Following the principle of least surprise, these three cases should behave equally, since they constrain the matched type equally:

case Composite[Int](v) =>
case Composite(v: Int) =>
case Composite[Int](v: Int) =>

But this is a good stop-gap solution however.

@odersky
Copy link
Contributor Author

odersky commented Jul 13, 2022

Following the principle of least surprise, these three cases should behave equally, since they constrain the matched type equally:

I am afraid the theory of GADTs is a minefield, and full of surprises. So least surprise would be nice, but unattainable. And it's not just theory either. Every misstep is backed by a test throwing a ClasscastException.

@dwijnand
Copy link
Member

My reading is that it's not a compile error, it's a made-fatal unchecked warning, no?

Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the details, I hadn't seen those PRs before.

@odersky odersky merged commit 9fdcfd2 into scala:main Jul 14, 2022
@odersky odersky deleted the fix-15662 branch July 14, 2022 15:03
@Kordyjan Kordyjan added this to the 3.2.1 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

java.lang.ClassCastException on a trivial match
5 participants