Skip to content

Should we remove scala.Enum trait as a parent of enums? #9873

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

Closed
bishabosha opened this issue Sep 24, 2020 · 4 comments
Closed

Should we remove scala.Enum trait as a parent of enums? #9873

bishabosha opened this issue Sep 24, 2020 · 4 comments

Comments

@bishabosha
Copy link
Member

bishabosha commented Sep 24, 2020

This issue poses the question of do we remove this trait, but keep the same API for individual enum classes, e.g. the following would still work after the change:

Minimized example

enum Colour extends java.lang.Enum[Colour]:
  case Red, Green, Blue

@main def Test = 
  println(Colour.Red.enumLabel)
  println(Colour.Green.ordinal)
  println(Colour.Blue.isInstanceOf[scala.Enum])

Output

Red
1
false

Points in Favour

  • If we remove the trait, then there is less overhead in the library, and there is no need to fully qualify java.lang.Enum
  • we still have Mirror framework which can be used to extract the ordinal generically

Points Against

  • currently in desugaring there is no way to determine in the statements of abstract class Colour if it derives from java.lang.Enum (Edit: alternatively it is possible to special case some specific desugarings to occur after parents are known)
  • therefore we can not avoid defining abstract def ordinal: Int when the parent is java.lang.Enum, leading to override errors
  • Mirror framework does not have generic way to fetch enumLabel
  • scala.Enum could be renamed (and remain a parent) so that java.lang.Enum parent does not need to be fully qualified

Considerations

  • perhaps the ordinal method should be renamed to avoid clashes with java.lang.Enum
  • perhaps we generate no public ordinal instance method and advise to use the ordinal method on the companion object. (same for enumLabel?)
  • perhaps revise Mirror framework to have a label method for Mirror.Sum

cc @odersky @smarter @sjrd

@bishabosha
Copy link
Member Author

alternatively it is possible to special case some specific desugarings to occur after parents are known

@julienrf
Copy link
Contributor

  • Mirror framework does not have generic way to fetch enumLabel

I think this is a problem. Could we have a way to provide the label to the Mirror framework without relying on scala.Enum? How do we currently get the labels of case object extending sealed traits?

  • If we remove the trait, then there is less overhead in the library, and there is no need to fully qualify java.lang.Enum

Yes but scala.Enum can’t be removed if we guarantee backward compatibility.

@bishabosha
Copy link
Member Author

bishabosha commented Sep 25, 2020

I think this is a problem. Could we have a way to provide the label to the Mirror framework without relying on scala.Enum? How do we currently get the labels of case object extending sealed traits?

This one is fine because the code that is already generated checks for the enum flag, and knows the concrete class so this is just an extra case.

Looking at the tests again, it appears it is possible that given a Mirror.ProductOf, you can then use the MirroredLabel type to retrieve the label, but then this does need to be an inline def, so we should have a runtime equivalent: https://github.com/lampepfl/dotty/blob/2ec7a6f8bb67168d23b744d62e274c17c55db5aa/tests/run/typeclass-derivation3.scala#L205

Yes but scala.Enum can’t be removed if we guarantee backward compatibility.

This should be fine as we've removed other things, like when we changed the package of scala.util.Not

bishabosha added a commit to dotty-staging/dotty that referenced this issue Sep 25, 2020
…nums

this commit introduces the concept that some desugaring expansion
can occur after parents of the current class are known, i.e. methods
that contain no references like ordinal and enumLabel.
bishabosha added a commit to dotty-staging/dotty that referenced this issue Sep 25, 2020
…nums

this commit introduces the concept that some desugaring expansion
can occur after parents of the current class are known, i.e. methods
that contain no references like ordinal and enumLabel.
bishabosha added a commit to dotty-staging/dotty that referenced this issue Oct 5, 2020
scala.reflect.Enum is now a universal super trait.

Also avoid using derivesFrom(defn.EnumClass) and
instead look for enum flag.
bishabosha added a commit to dotty-staging/dotty that referenced this issue Oct 5, 2020
scala.reflect.Enum is now a universal super trait.

Also avoid using derivesFrom(defn.EnumClass) and
instead look for enum flag.
bishabosha added a commit to dotty-staging/dotty that referenced this issue Oct 5, 2020
scala.reflect.Enum is now a universal super trait.

Also avoid using derivesFrom(defn.EnumClass) and
instead look for enum flag.
@bishabosha
Copy link
Member Author

bishabosha commented Oct 5, 2020

Spec given: We will rename scala.Enum to scala.reflect.Enum instead, and make it a super trait

bishabosha added a commit that referenced this issue Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants