-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #5067: handle scrutinee of bottom type in pattern matcher #5075
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
Conversation
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.
Adding a new kind of plan to the pattern matcher is a fairly big change/complication. To do this "just" to make the compiler not crash for a non-sensical case might go too far. I have a tendency to do what scalac does and disallow these patterns.
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.
@abeln Can you change the PR to outlaw these situations instead? Thanks!
Here is another suggestion that does not require another plan:
|
@odersky yes, I'll take a look. Got busy preparing the proposal for explicit nulls! |
When desugaring pattern matching code for expressions where the matched value has type `Null` or `Nothing`, we used to generate code that's type-incorrect. Example: ``` val Some(x) = null ``` got desugared into ``` val x: Nothing = matchResult1[Nothing]: { case val x1: Null @unchecked = null: Null @unchecked if x1.ne(null) then { case val x: Nothing = x1.value.asInstanceOf[Nothing] return[matchResult1] x: Nothing } else () return[matchResult1] throw new MatchError(x1) } ``` There were two problems here: 1) `x1.ne(null)` 2) `x1.value` In both cases, we're trying to invoke methods that don't exist for type `Nothing` (and #2 doesn't exist for `Null`). This commits changes the desugaring so that 1) is solved by adding an ascription, if needed: (x1: AnyRef).ne(null) 2) is added by generating throw-away but type-correct code that never executes: `throw null`
72ab3af
to
2007ad2
Compare
@odersky PTAL |
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.
My main concern is the duplication in isSubtypeOfBottom
.
Flip some conditions. Remove isSubtypeOfBottom. Add new tests.
@odersky PTAL |
To desugar constructs like ``` val Some(_) = null ``` this uses the approach in scala#5075 as opposed to the older approach that used NoOpPlans.
@abeln Looks good now. |
When desugaring pattern matching code for expressions where the
matched value has type
Null
orNothing
, we used to generate codethat's type-incorrect.
Example:
got desugared into
There were two problems here:
x1.ne(null)
x1.value
In both cases, we're trying to invoke methods that don't exist for type
Nothing
(and #2 doesn't exist forNull
).This commits changes the desugaring so that
executes:
throw null