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.

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

Fix #6314: match type unsoundness #6319

Merged
merged 11 commits into from
May 17, 2019

Conversation

OlivierBlanvillain
Copy link
Contributor

@OlivierBlanvillain OlivierBlanvillain commented Apr 16, 2019

This PR fixes several match issues

The first issue was that the algorithm assumes types in an AndType always came in the same order, so it wrongly concluded that X & Y and Y & Type are non intersecting from the fact that X and Y are non intersecting. The updated logic checks both permutations, and simply get stuck when Type is abstract, which fixes the unsoundness.

The second issue is related to the Singleton. This marker type needs to be taken it into account by the intersection algorithm, see the added test case.

Finally the previous handling of abstract type was problematic a showed by several of the examples in #6314. The logic is updated to do a sub-type check where all the abstract types (members or parameters) are replace by WildcardType. If the scrutinee is not a subtype of a pattern, but it is with wildcards, then the type shouldn't reduce.

Seams to be a leftover from scalac old pattern matcher and has always
been dead code in dotty.
Singleton is not part of the normal type hierarchy, special
treatment in requiered in intersecting to avoid making incorrect
assumptions.
It timed out on my machine...
This breaks tests/run/typeclass-derivation2c.scala
There is no way to statically know that the type passed to constValue reduces.
@OlivierBlanvillain OlivierBlanvillain changed the title Fix #6314: don't assume types in & are sorted Fix #6314: match type unsoundness Apr 30, 2019
@odersky odersky assigned OlivierBlanvillain and unassigned odersky May 7, 2019
SeklomType are already mapped over correctly, that is, as .info.
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.

None yet

2 participants