-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Keep opaques in match type scrutinee #20033
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
base: main
Are you sure you want to change the base?
Conversation
@@ -3488,7 +3488,7 @@ class MatchReducer(initctx: Context) extends TypeComparer(initctx) { | |||
false | |||
|
|||
case MatchTypeCasePattern.AbstractTypeConstructor(tycon, argPatterns) => | |||
scrut.dealias match | |||
scrut.dealiasKeepOpaques 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.
This change is direct contradiction with the spec, and will break as many things as it will fix (since another use case might be to precisely take private knowledge into account to be able to perform a reduction).
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.
If I understood correctly, the following
object OpaqueScope:
opaque type Opaque[N <: Any] = Double
val foo: StripOpaque[Opaque[Int]] = 1
// Note that match types within OpaqueScope are not limited by the absence of dealiasing
type NeedsRhs[D] = D match
case Double => String
val bar: NeedsRhs[Opaque[Float]] = "hello"
is an example of what you are mentioning right ?
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.
Yes, exactly.
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 still works, and is part of the 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.
Oh yes because you only changed the dealiasing rule for the AbstractTypeConstructor
case. You have to complicate the test a bit more so that it would also fall into that case to observe the issue.
Fixes #19326