Skip to content

Fix #9011: Make single enum values inherit from Product #9018

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 12 commits into from
May 22, 2020
8 changes: 5 additions & 3 deletions compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ object DesugarEnums {

/** A creation method for a value of enum type `E`, which is defined as follows:
*
* private def $new(_$ordinal: Int, $name: String) = new E {
* private def $new(_$ordinal: Int, $name: String) = new E with EnumValue {
* def $ordinal = $tag
* override def toString = $name
* $values.register(this)
Expand All @@ -135,7 +135,7 @@ object DesugarEnums {
val toStringDef = toStringMeth(Ident(nme.nameDollar))
val creator = New(Template(
constr = emptyConstructor,
parents = enumClassRef :: Nil,
parents = enumClassRef :: scalaDot(str.EnumValue.toTypeName) :: Nil,
derived = Nil,
self = EmptyValDef,
body = List(ordinalDef, toStringDef) ++ registerCall
Expand Down Expand Up @@ -286,7 +286,9 @@ object DesugarEnums {
val (tag, scaffolding) = nextOrdinal(CaseKind.Object)
val ordinalDef = ordinalMethLit(tag)
val toStringDef = toStringMethLit(name.toString)
val impl1 = cpy.Template(impl)(body = List(ordinalDef, toStringDef) ++ registerCall)
val impl1 = cpy.Template(impl)(
parents = impl.parents :+ scalaDot(str.EnumValue.toTypeName),
body = List(ordinalDef, toStringDef) ++ registerCall)
.withAttachment(ExtendsSingletonMirror, ())
val vdef = ValDef(name, TypeTree(), New(impl1)).withMods(mods.withAddedFlags(EnumValue, span))
flatTree(scaffolding ::: vdef :: Nil).withSpan(span)
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/core/StdNames.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ object StdNames {

final val MODULE_INSTANCE_FIELD = "MODULE$"

final val EnumValue = "EnumValue"
final val Function = "Function"
final val ErasedFunction = "ErasedFunction"
final val ContextFunction = "ContextFunction"
Expand Down
10 changes: 10 additions & 0 deletions library/src/scala/EnumValue.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package scala

trait EnumValue extends Product:
Copy link
Member

Choose a reason for hiding this comment

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

For parity with case object this should extend Serializable too I think:

Suggested change
trait EnumValue extends Product:
trait EnumValue extends Product with Serializable:

Copy link
Member

Choose a reason for hiding this comment

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

Actually it probably makes more sense to have Product and Serializable added as parents to the enum class itself, that way they never show up when lubbing enum cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but I am not sure about what other effects this will have.

override def canEqual(that: Any) = true
override def productArity: Int = 0
override def productPrefix: String = toString
override def productElement(n: Int): Any =
throw IndexOutOfBoundsException(n.toString())
override def productElementName(n: Int): String =
throw IndexOutOfBoundsException(n.toString())
57 changes: 57 additions & 0 deletions tests/run/i9011.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
enum Opt[+T] derives Eq:
case Sm(t: T)
case Nn

import scala.deriving._
import scala.compiletime.{erasedValue, summonInline}

trait Eq[T] {
def eqv(x: T, y: T): Boolean
}

object Eq {
given Eq[Int] {
def eqv(x: Int, y: Int) = x == y
}

inline def summonAll[T <: Tuple]: List[Eq[_]] = inline erasedValue[T] match {
case _: Unit => Nil
case _: (t *: ts) => summonInline[Eq[t]] :: summonAll[ts]
}

def check(elem: Eq[_])(x: Any, y: Any): Boolean =
elem.asInstanceOf[Eq[Any]].eqv(x, y)

def iterator[T](p: T) = p.asInstanceOf[Product].productIterator
Copy link
Member

@smarter smarter May 22, 2020

Choose a reason for hiding this comment

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

Going back to the original issue (#9011), is this cast actually safe?

The example in the docs suggests that if we have a Mirror.ProductOf[T] then we can safely cast an instance of T to Product (in order to access productIterator etc.). But this is not the case.

If this isn't actually guaranteed, then the documentation needs to be updated to not rely on this cast in an example.


def eqSum[T](s: Mirror.SumOf[T], elems: List[Eq[_]]): Eq[T] =
new Eq[T] {
def eqv(x: T, y: T): Boolean = {
val ordx = s.ordinal(x)
(s.ordinal(y) == ordx) && check(elems(ordx))(x, y)
}
}

def eqProduct[T](p: Mirror.ProductOf[T], elems: List[Eq[_]]): Eq[T] =
new Eq[T] {
def eqv(x: T, y: T): Boolean =
iterator(x).zip(iterator(y)).zip(elems.iterator).forall {
case ((x, y), elem) => check(elem)(x, y)
}
}

inline given derived[T](using m: Mirror.Of[T]) as Eq[T] = {
val elemInstances = summonAll[m.MirroredElemTypes]
inline m match {
case s: Mirror.SumOf[T] => eqSum(s, elemInstances)
case p: Mirror.ProductOf[T] => eqProduct(p, elemInstances)
}
}
}

object Test extends App {
import Opt._
val eqoi = summon[Eq[Opt[Int]]]
assert(eqoi.eqv(Sm(23), Sm(23)))
assert(eqoi.eqv(Nn, Nn))
}