-
Notifications
You must be signed in to change notification settings - Fork 3.1k
scala/bug#4364. Mention null in extractor pattern #6374
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
Closed
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spec/08-pattern-matching.md
Outdated
| } | ||
| val z = null match { | ||
| case Pair(_, _) => "a pair" | ||
| caes _ => "not a pair" |
Contributor
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.
caes -> case
Member
Author
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.
Thanks.
Ref scala/bug#4364 The specification clarification that is needed is this: What should the pattern matcher do when encountering a `null` and an extractor pattern? As it stands, SLS does not say anything about `null` for extractor pattern, which leads: - Pattern matcher will happily pass `null` into your extractor. - I can write a `null`-matching extractor MyNull. - Thus, all extractor authors must check for `null` if they would like to access the passed in argument `x`. See `Animal` example below. There are potentially two things we can do. 1. Be ok with the status quo. Make sure we handle `null` in all our extractors. 2. Not be ok with the status quo. Suppose we would want extractor pattern to be non-null values only, then that's a spec change and change in pattern matcher's behavior. It might also break existing code that handled `null` in ever so subtle way. I think Paul is suggesting option 1 in scala/bug#4364, not spec change. Given that there are no outcry of `null` handling besides scala/bug#2241, I think that's fine too. ### MyNull ```scala scala> object MyNull { | def unapply(x: AnyRef): Option[AnyRef] = | if (x eq null) Some(x) else None | } defined object MyNull scala> null match { case MyNull(_) => "yes"; case _ => "no" } res4: String = yes ``` ### Animal ```scala scala> object Animal { | def unapply(x: AnyRef): Option[AnyRef] = | if (x.toString == "Animal") Some(x) | else None | } defined object Animal scala> null match { case Animal(_) => "yes"; case _ => "no" } java.lang.NullPointerException at Animal$.unapply(<console>:13) ... 37 elided ```
0600275 to
3e698f4
Compare
Contributor
|
Another sample outcry: scala/bug#8787 |
eed3si9n
added a commit
to eed3si9n/scala
that referenced
this pull request
Apr 1, 2018
Ref scala/bug#4364 This is a general fix for scala/bug#2241 and scala/bug#8787. scala/bug#4364 raises the question of what should the pattern matcher do when encountering a `null` and an extractor pattern. As it stands, SLS does not say anything about `null` for extractor pattern, which leads: - Pattern matcher will happily pass `null` into your extractor. - I can write a `null`-matching extractor MyNull. - Thus, all extractor authors must check for `null` if they would like to access the passed in argument `x`. There are potentially two things we can do. 1. Be ok with the status quo. Clarify in SLS that all extractor authors must handle `null`. 2. Not be ok with the status quo. Change SLS and implementation such that `null` is treated as no match (empty). I've already sent option 1 as scala#6374. This implements option 2.
This was referenced Apr 1, 2018
eed3si9n
added a commit
to eed3si9n/scala
that referenced
this pull request
Apr 1, 2018
Ref scala/bug#4364 This is a general fix for scala/bug#2241 and scala/bug#8787. scala/bug#4364 raises the question of what should the pattern matcher do when encountering a `null` and an extractor pattern. As it stands, SLS does not say anything about `null` for extractor pattern, which leads: - Pattern matcher will happily pass `null` into your extractor. - I can write a `null`-matching extractor MyNull. - Thus, all extractor authors must check for `null` if they would like to access the passed in argument `x`. There are potentially two things we can do. 1. Be ok with the status quo. Clarify in SLS that all extractor authors must handle `null`. 2. Not be ok with the status quo. Change SLS and implementation such that `null` is treated as no match (empty). I've already sent option 1 as scala#6374. This implements option 2.
Contributor
|
Closing in favor of status co-quo. |
eed3si9n
added a commit
to eed3si9n/scala
that referenced
this pull request
Apr 12, 2018
Ref scala/bug#4364 This is a general fix for scala/bug#2241 and scala/bug#8787. scala/bug#4364 raises the question of what should the pattern matcher do when encountering a `null` and an extractor pattern. As it stands, SLS does not say anything about `null` for extractor pattern, which leads: - Pattern matcher will happily pass `null` into your extractor. - I can write a `null`-matching extractor MyNull. - Thus, all extractor authors must check for `null` if they would like to access the passed in argument `x`. There are potentially two things we can do. 1. Be ok with the status quo. Clarify in SLS that all extractor authors must handle `null`. 2. Not be ok with the status quo. Change SLS and implementation such that `null` is treated as no match (empty). I've already sent option 1 as scala#6374. This implements option 2.
eed3si9n
added a commit
to eed3si9n/scala
that referenced
this pull request
May 4, 2018
Ref scala/bug#4364 This is a general fix for scala/bug#2241 and scala/bug#8787. scala/bug#4364 raises the question of what should the pattern matcher do when encountering a `null` and an extractor pattern. As it stands, SLS does not say anything about `null` for extractor pattern, which leads: - Pattern matcher will happily pass `null` into your extractor. - I can write a `null`-matching extractor MyNull. - Thus, all extractor authors must check for `null` if they would like to access the passed in argument `x`. There are potentially two things we can do. 1. Be ok with the status quo. Clarify in SLS that all extractor authors must handle `null`. 2. Not be ok with the status quo. Change SLS and implementation such that `null` is treated as no match (empty). I've already sent option 1 as scala#6374. This implements option 2.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ref scala/bug#4364
The specification clarification that is needed is this: What should the pattern matcher do when encountering a
nulland an extractor pattern?As it stands, SLS does not say anything about
nullfor extractor pattern, which leads:nullinto your extractor.null-matching extractor MyNull.nullif they would like to access the passed in argumentx. SeeAnimalexample below.There are potentially two things we can do.
I think Paul is suggesting option 1 in scala/bug#4364, not spec change. Given that there are no outcry of
nullhandling besides scala/bug#2241, I think that's fine too.MyNull
Animal