Skip to content

fix #10511: compare enumvalues in provably disjoint #10938

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

Merged
merged 5 commits into from
Jan 11, 2021

Conversation

bishabosha
Copy link
Member

@bishabosha bishabosha commented Dec 28, 2020

this also shortcuts comparing object refs without looking at their supertype

fixes #10511

@bishabosha bishabosha force-pushed the match-tpe-enums branch 3 times, most recently from a6b174c to 6676d14 Compare December 28, 2020 11:44
@bishabosha
Copy link
Member Author

bishabosha commented Dec 28, 2020

I have also tested transitivity of passing aliases to singleton enum values, such as this:

val t: True.type = True
val f: False.type = False

val t1: Not[f.type] = t // transitivity
val f1: Not[t.type] = f // transitivity

Edit: this has been prevented from running in the FromTasty test due to aparrently different reduction of match types with that option

@bishabosha bishabosha added this to the 3.0.0-RC1 milestone Dec 29, 2020
case False.type => True.type
}

def not[B <: Bool & Singleton](b: B): Not[B] = b match {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's nice that Singleton helps inference here, the following compiles as expected:

not(True): False.type

Although it's a bit fragile:

val vrais = True
not(vrais): False.type // error, we lost the singleton type...

Copy link
Member Author

@bishabosha bishabosha Dec 30, 2020

Choose a reason for hiding this comment

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

yeah its true with enums a user must do a lot of work to avoid the widening but thats something hopefully we can tune based on feedback without ruining bincompat

@OlivierBlanvillain
Copy link
Contributor

I think this PR should also update the docs, the diff LGTM otherwise.

It wouldn't be sounds to compare singleton types in the general case, but as far as I can tell it's fine to do for enums because value cases are guaranteed to point to a different object.

@bishabosha
Copy link
Member Author

I've updated the match-types doc

@bishabosha
Copy link
Member Author

bishabosha commented Dec 30, 2020

so FromTastyTest for pos/i10511.scala failed at -Ycheck:readTasty due to the match type Not[(f1 : Not[(t : (Bool.True : Bool))])] not reducing under -from-tasty, I've added an exception to FromTastyTest for now

@bishabosha
Copy link
Member Author

bishabosha commented Dec 30, 2020

it is possible to get the -from-tasty -Ycheck:readTasty for Not[(f1 : Not[(t : (Bool.True : Bool))])] to pass if I call sym.info.normalized in a recursive version of isEnumValueOrModule but it is probably best to do that at unpickling time when we see a match type:

def isEnumValueOrModule(ref: TermRef): Boolean =
  val sym = ref.termSymbol
  sym.isAllOf(EnumCase, butNot=JavaDefined)
    || sym.is(Module)
    || (sym.info.normalized match
        case tp: TermRef => isEnumValueOrModule(tp)
        case tp => false)

or get Ycheck to normalise types before checking bounds

@bishabosha bishabosha requested review from smarter and removed request for smarter and odersky January 4, 2021 15:03
@bishabosha bishabosha removed the request for review from smarter January 4, 2021 15:03
@OlivierBlanvillain OlivierBlanvillain merged commit ea87402 into scala:master Jan 11, 2021
@OlivierBlanvillain OlivierBlanvillain deleted the match-tpe-enums branch January 11, 2021 15:42
@Kordyjan Kordyjan modified the milestones: 3.0.0-RC1, 3.0.0 Aug 2, 2023
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.

Enum Value singleton fails to reduce in match type
5 participants