-
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 sealed prefixes in exh checking #16621
Conversation
96653a9
to
27be4f2
Compare
391b433
to
0e52250
Compare
tests/pos/i15029.more.scala
Outdated
case X.Field() => | ||
case X.Thing => | ||
case Y.Field() => | ||
case Y.Thing => |
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.
In the example mentioned above, the code is as follows:
def handle(schema: Schema[String]) =
schema match // was: match may not be exhaustive
case Y.Field() =>
case Y.Thing =>
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.
So, in that case it still does the right thing, though the showType
logic isn't handling it very well.
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.
Sorry, I was not clear. If I run the following example with this PR:
// scalac: -Werror
sealed trait Schema[A]
sealed class Outer:
sealed trait RecordInstances[B]:
case class Field() extends Schema[B]
case object Thing extends Schema[B]
object O1 extends Outer
object O2 extends Outer
object X extends O1.RecordInstances[Int]
object Y extends O2.RecordInstances[String]
// Match not exhaustive error! (with fatal warnings :P)
class Test:
def handle(schema: Schema[String]) =
schema match // was: match may not be exhaustive
case Y.Field() =>
case Y.Thing =>
I get the following warning:
-- [E029] Pattern Match Exhaustivity Warning: tests/pos/i15029.more.scala:18:4 -
18 | schema match // was: match may not be exhaustive
| ^^^^^^
| match may not be exhaustive.
|
| It would fail on pattern case: X.Field(), Thing
|
| longer explanation available when compiling with `-explain`
I expect no warnings in this case.
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.
Ah, my mistake, didn't see the Schema[String]
either time.
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.
Excellent, found a simplier way to do it, which makes your more complicated version work! ❤️
0e52250
to
f9ed263
Compare
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.
Elegant fix, LGTM 👍
f9ed263
to
0dae27d
Compare
No description provided.