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

Fix #12485: Avoid cycles in testing feasibility of exhaustivity check #12488

Merged
merged 1 commit into from
May 17, 2021
Merged

Fix #12485: Avoid cycles in testing feasibility of exhaustivity check #12488

merged 1 commit into from
May 17, 2021

Conversation

liufengyun
Copy link
Contributor

Fix #12485: Avoid cycles in testing feasibility of exhaustivity check

This is a regression introduced in #12377.

@liufengyun liufengyun requested a review from dwijnand May 17, 2021 13:44
dwijnand
dwijnand approved these changes May 17, 2021
View reviewed changes
Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Does classSym.is(Case) always imply defn.isProductSubType(tpw), even for big case classes? productSelectorTypes must be doing something sensible.

@liufengyun
Copy link
Contributor Author

Thanks @dwijnand !

LGTM. Does classSym.is(Case) always imply defn.isProductSubType(tpw), even for big case classes? productSelectorTypes must be doing something sensible.

Indeed, I just checked productSelectorTypes, if it's not a product, then it returns an emty list.

@liufengyun liufengyun merged commit 1283819 into scala:master May 17, 2021
@liufengyun liufengyun deleted the fix-12485 branch May 17, 2021 14:31
@dwijnand dwijnand mentioned this pull request Jul 8, 2021
Merged
dwijnand added a commit to dwijnand/scala3 that referenced this pull request Jul 8, 2021
@dwijnand
Exhaustivity warnings on nested case classes
d8c4ad8 
Fixes scala#13003, by refixing scala#12485 (PR scala#12488).

Part of the issue is that isCheckable behaves differently under
-Ycheck-all-patmat and our tests only run under that flag.  So for
starters I added a test variant where that flag isn't used.  I'd like to
understand why that flag exists to see if we could remove it from
guarding the logic and the tests.
dwijnand added a commit to dwijnand/scala3 that referenced this pull request Jul 9, 2021
@dwijnand
Exhaustivity warnings on nested case classes
9fd4000 
Fixes scala#13003, by refixing scala#12485 (PR scala#12488).

Part of the issue is that isCheckable behaves differently under
-Ycheck-all-patmat and our tests only run under that flag.  So for
starters I added a test variant where that flag isn't used.  I'd like to
understand why that flag exists to see if we could remove it from
guarding the logic and the tests.

Also make sure that any patmat test that throws an exception fails the
test...
BarkingBad pushed a commit to BarkingBad/dotty that referenced this pull request Jul 23, 2021
@dwijnand @BarkingBad
Exhaustivity warnings on nested case classes
367bdf3 
Fixes scala#13003, by refixing scala#12485 (PR scala#12488).

Part of the issue is that isCheckable behaves differently under
-Ycheck-all-patmat and our tests only run under that flag.  So for
starters I added a test variant where that flag isn't used.  I'd like to
understand why that flag exists to see if we could remove it from
guarding the logic and the tests.

Also make sure that any patmat test that throws an exception fails the
test...
@Kordyjan Kordyjan added this to the 3.0.1 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dwijnand dwijnand dwijnand approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.0.1
Development

Successfully merging this pull request may close these issues.

stackoverflow during scala script compile
3 participants
@liufengyun @dwijnand @Kordyjan