Check all bindings exhaustively, e.g. tuples components #9147
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
lrytz
approved these changes
Aug 18, 2020
Member
lrytz
left a comment
lrytz
left a comment
There was a problem hiding this comment.
LGTM for what I can say - this area of the compiler is completely new to me and I didn't spend enough time to understand it in detail.
The change to drop the null checks (introducing `TreeMakersToPropsIgnoreNullChecks`) over-reached and got rid of all the Vars that those null-checks were referencing. So, instead of that, finish honouring the `modelNull` option in `removeVarEq`.
dwijnand
force-pushed
the
exhaust-tuple
branch
from
522e899 to
93d1a5e
Compare
dongjoon-hyun
added a commit
to apache/spark
that referenced
this pull request
Nov 24, 2020
### What changes were proposed in this pull request? This PR aims the followings. 1. Upgrade from Scala 2.13.3 to 2.13.4 for Apache Spark 3.1 2. Fix exhaustivity issues in both Scala 2.12/2.13 (Scala 2.13.4 requires this for compilation.) 3. Enforce the improved exhaustive check by using the existing Scala 2.13 GitHub Action compilation job. ### Why are the changes needed? Scala 2.13.4 is a maintenance release for 2.13 line and improves JDK 15 support. - https://github.com/scala/scala/releases/tag/v2.13.4 Also, it improves exhaustivity check. - scala/scala#9140 (Check exhaustivity of pattern matches with "if" guards and custom extractors) - scala/scala#9147 (Check all bindings exhaustively, e.g. tuples components) ### Does this PR introduce _any_ user-facing change? Yep. Although it's a maintenance version change, it's a Scala version change. ### How was this patch tested? Pass the CIs and do the manual testing. - Scala 2.12 CI jobs(GitHub Action/Jenkins UT/Jenkins K8s IT) to check the validity of code change. - Scala 2.13 Compilation job to check the compilation Closes #30455 from dongjoon-hyun/SCALA_3.13. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
finaglehelper
pushed a commit
to twitter/util
that referenced
this pull request
Jan 29, 2021
Problem Supporting latest https://github.com/scala/scala/releases/tag/v2.12.13 includes a feature change in the scala compiler that will fail to compile when case matching is not exhaustive on tuples. Issues - scala/bug#10680 - scala/bug#10373 - scala/bug#10019 Resolved - scala/scala#9163 - scala/scala#9147 Solution Add `case _ => throw new MatchError` to case matching statement that is non-exhaustive JIRA Issues: SCALA-25 Differential Revision: https://phabricator.twitter.biz/D609046
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
With this fix the exhaustivity checker now emits exhaustivity warnings for tuples (and possibly other pattern matches that were affected).
Fixes scala/bug#10019
Technically
The change to drop the null checks (introducing
TreeMakersToPropsIgnoreNullChecks) over-reached and got rid of all theVars that those null-checks were referencing. So, instead of that,
finish honouring the
modelNulloption inremoveVarEq.