Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Switch off exhaustivity warnings for unsealed types (by default) #9299

Merged
merged 1 commit into from Nov 5, 2020
Merged

Switch off exhaustivity warnings for unsealed types (by default) #9299

merged 1 commit into from Nov 5, 2020

Conversation

dwijnand
Copy link
Member

@dwijnand dwijnand commented Nov 3, 2020 •
edited by SethTisue

Replaces -Xno-unsealed-patmat-analysis with -Xlint:strict-unsealed-patmat, so that you must opt-in if you want this strictness.

:(

However, at least by being in the Xlint family, it's on if you opt-in to -Xlint.

Reverts commit 0416c57 (w/ tweaks).

@dwijnand
Switch off exhaustivity warnings for unsealed types (by default)
35d9bda 
Replaces `-Xno-unsealed-patmat-analysi` with `-Xlint:strict-unsealed-patmat`, switching the default behaviour to opt-in to this strictness.

:(

However, at least by being in the Xlint family, it's on if you opt-in to `-Xlint`.

Reverts commit 0416c57 (w/ tweaks).
@scala-jenkins scala-jenkins added this to the 2.13.5 milestone Nov 3, 2020
@dwijnand dwijnand modified the milestones: 2.13.5, 2.13.4 Nov 3, 2020
@dwijnand dwijnand added the release-notes worth highlighting in next release notes label Nov 3, 2020
@dwijnand
Copy link
Member Author

dwijnand commented Nov 3, 2020

🤔 I'll fold this change into #9140, but perhaps this PR should be a link in the "exhaustivity" section of the release notes.

@dwijnand dwijnand mentioned this pull request Nov 3, 2020
Merged
@retronym
Copy link
Member

retronym commented Nov 3, 2020

patmat is something of an insider's term for the pattern matcher compiler phase. How about something strict-unsealed-patterns instead? Sorry to 🚲 🏚️ ...

@dwijnand
Copy link
Member Author

dwijnand commented Nov 3, 2020

Sure. We do also have -Xno-patmat-analysis, -Vpatmat, -Ypatmat-exhaust-depth, -Vstatistics:patmat/-Vprint:patmat, but I guess -Xlint is even more general user-facing than those.

@retronym
Copy link
Member

retronym commented Nov 3, 2020

Hmmm.... let's leave it as is in this PR.

@dwijnand dwijnand mentioned this pull request Nov 3, 2020
Closed
72 tasks
@sjrd sjrd mentioned this pull request Nov 3, 2020
Closed
@SethTisue
Copy link
Member

SethTisue commented Nov 3, 2020 •
edited

to repeat in public some points from our internal discussions:

Martin and Guillaume both expressed reservations about making this warn by default, for now anyway, especially since the changes to the pattern matcher are so new so we don't have much experience with them yet. Going forward, we'll want to coordinate with the Scala 3 team about any further changes to defaults (e.g. on https://contributors.scala-lang.org where others can weigh in as well).

Making it part of the -Xlint defaults seems like a pretty good compromise in that just about anybody who cares about code quality should be using -Xlint and a great many people actually do, so this is still gives the new warnings fairly prominent placement, so we will probably still get good feedback from users this way.

Note also that over at #7525 @som-snytt is exploring ideas such as perhaps backporting Scala 3's case syntax for marking matches as might-fail.

@SethTisue SethTisue requested a review from lrytz November 4, 2020 16:32
@dwijnand dwijnand merged commit e6a6ac6 into scala:2.13.x Nov 5, 2020
@dwijnand dwijnand deleted the xlint-unsealed-match branch November 5, 2020 08:56
sjrd added a commit to sjrd/scala-js that referenced this pull request Nov 5, 2020
@sjrd
Address new warnings with the stricter patmat exhaustivity of 2.13.4.
1d7e1dc 
In 2.13.4, the pattern match exhaustivity analysis has become
stricter. It remains active in the presence of guards and custom
extractors (including `unapplySeq`). This change causes some new
warnings in our codebase, which this commit addresses.

See scala/scala#9140, which was somewhat
toned down by scala/scala#9299.
@sjrd sjrd mentioned this pull request Nov 5, 2020
Merged
sjrd added a commit to sjrd/scala-js that referenced this pull request Nov 7, 2020
@sjrd
Address new warnings with the stricter patmat exhaustivity of 2.13.4.
592391e 
In 2.13.4, the pattern match exhaustivity analysis has become
stricter. It remains active in the presence of guards and custom
extractors (including `unapplySeq`). This change causes some new
warnings in our codebase, which this commit addresses.

See scala/scala#9140, which was somewhat
toned down by scala/scala#9299.
@sjrd
Copy link
Member

sjrd commented Nov 8, 2020

Could someone trigger an update of https://raw.githubusercontent.com/scala/community-builds/2.13.x/nightly.properties now that this PR has been merged, please? That would allow to restore our nightly-against-nightly build, to make sure we're not hitting something else among the recent changes.

@SethTisue
Copy link
Member

SethTisue commented Nov 8, 2020 •
edited

@sjrd that's already in flight at scala/community-build#1284 — it’s nearly certain to cross the finish line in the next 1-2 days

@SethTisue
Copy link
Member

whoops, I got confused — 1284 is about project SHAs, not Scala SHAs. I made a new PR advancing the Scala SHA: scala/community-build#1286

@dwijnand dwijnand removed the release-notes worth highlighting in next release notes label Nov 10, 2020
@Christewart Christewart mentioned this pull request Nov 26, 2020
Open
@smarter smarter mentioned this pull request Jun 18, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lrytz lrytz lrytz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.13.4
6 participants
@dwijnand @retronym @SethTisue @sjrd @lrytz @scala-jenkins