Skip to content

Fix TypeTest exhaustivity check #12036

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

nicolasstucki
Copy link
Contributor

Properly capture the semantics of TypeTest in the Space logic.
TypeTest[S, T].unapply is equivalent to (and mostly used as) a _: T pattern and therefore covers all T.

@nicolasstucki
Copy link
Contributor Author

This can be later replaced with the generalized covering checks in #11186.

Properly capture the semantics of `TypeTest` in the `Space` logic.
`TypeTest[S, T].unapply` is equivalent to (and mostly used as) a `_: T` pattern and therefore covers all `T`.

* Update the documentation to make this feature clearer
* Fixes scala#12026
* Fixes scala#12020
* Improves scala#11541
@nicolasstucki nicolasstucki force-pushed the fix-TypeTest-exhausivity-check branch from f611fdc to 88de390 Compare April 9, 2021 08:16
@nicolasstucki nicolasstucki marked this pull request as ready for review April 9, 2021 10:01
@nicolasstucki nicolasstucki requested a review from liufengyun April 9, 2021 10:01
// If the scrutinee is not of type `S`, then we would already have an unchecked warning.
minus(a, params.head)
else
a // approximation
Copy link
Contributor

Choose a reason for hiding this comment

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

If the issues are really urgent to fix, I'd suggest extracting the code in #11186 (without introducing any annotations) and specialize the method covers instead. This way, we keep the logic in the base class general, which is a design invariant supposed to improve maintainability of the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@liufengyun could you help me with that?

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem, I'll create a new PR to do that today.

@nicolasstucki
Copy link
Contributor Author

Replaced with #12059

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeTest exhaustivity check Spurious exhaustivity warning on quote API + union types
2 participants