Skip to content

Re-introduce java.lang.Enum constructor hijack. #6624

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 1 commit into from
Jun 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,33 @@ class Definitions {
JavaSerializableClass.typeRef
else
ctx.requiredClassRef("scala.Serializable")

@threadUnsafe lazy val JavaEnumClass: ClassSymbol = {
val cls = ctx.requiredClass("java.lang.Enum")
Copy link
Member

Choose a reason for hiding this comment

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

ping @sjrd, this might interact with Scala.js reimplementation of Enum: https://github.com/scala-js/scala-js/blob/master/javalanglib/src/main/scala/java/lang/Enum.scala

Copy link
Member

Choose a reason for hiding this comment

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

Also it'd be nice to add a comment explaining why this hijacking is necessary.

Copy link
Member

Choose a reason for hiding this comment

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

Hum, it's unclear how that would interact with the Scala.js implementation, indeed. However, that's not an immediate problem, because for now Dotty.js reuses the binaries of java.lang.Enum.sjsir from Scala.js for scalac.

Worse come to worst, we would hard-code the .sjsir of java.lang.Enum in the distant future where we can actually compile Scala.js' standard library with Dotty.js.

cls.infoOrCompleter match {
case completer: ClassfileLoader =>
cls.info = new ClassfileLoader(completer.classfile) {
override def complete(root: SymDenotation)(implicit ctx: Context): Unit = {
super.complete(root)
val constr = cls.primaryConstructor
val newInfo = constr.info match {
case info: PolyType =>
info.resType match {
case meth: MethodType =>
info.derivedLambdaType(
resType = meth.derivedLambdaType(
paramNames = Nil, paramInfos = Nil))
}
}
constr.info = newInfo
constr.termRef.recomputeDenot()
}
}
cls
}
}
def JavaEnumType = JavaEnumClass.typeRef

def SerializableClass(implicit ctx: Context): ClassSymbol = SerializableType.symbol.asClass
@threadUnsafe lazy val StringBuilderType: TypeRef = ctx.requiredClassRef("scala.collection.mutable.StringBuilder")
def StringBuilderClass(implicit ctx: Context): ClassSymbol = StringBuilderType.symbol.asClass
Expand Down Expand Up @@ -674,8 +701,6 @@ class Definitions {
def NoneClass(implicit ctx: Context): ClassSymbol = NoneModuleRef.symbol.moduleClass.asClass
@threadUnsafe lazy val EnumType: TypeRef = ctx.requiredClassRef("scala.Enum")
def EnumClass(implicit ctx: Context): ClassSymbol = EnumType.symbol.asClass
@threadUnsafe lazy val JEnumType: TypeRef = ctx.requiredClassRef("scala.compat.JEnum")
def JEnumClass(implicit ctx: Context): ClassSymbol = JEnumType.symbol.asClass
@threadUnsafe lazy val EnumValuesType: TypeRef = ctx.requiredClassRef("scala.runtime.EnumValues")
def EnumValuesClass(implicit ctx: Context): ClassSymbol = EnumValuesType.symbol.asClass
@threadUnsafe lazy val ProductType: TypeRef = ctx.requiredClassRef("scala.Product")
Expand Down Expand Up @@ -1472,7 +1497,7 @@ class Definitions {
ScalaPackageClass.enter(m)

// force initialization of every symbol that is synthesized or hijacked by the compiler
val forced = syntheticCoreClasses ++ syntheticCoreMethods ++ ScalaValueClasses()
val forced = syntheticCoreClasses ++ syntheticCoreMethods ++ ScalaValueClasses() :+ JavaEnumClass

isInitialized = true
}
Expand Down
3 changes: 1 addition & 2 deletions compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -832,8 +832,7 @@ object JavaParsers {
AppliedTypeTree(javaLangDot(tpnme.Enum), List(enumType))
*/
val superclazz = Apply(TypeApply(
Select(New(javaLangDot(tpnme.Enum)), nme.CONSTRUCTOR), List(enumType)),
List(Literal(Constant(null)),Literal(Constant(0))))
Select(New(javaLangDot(tpnme.Enum)), nme.CONSTRUCTOR), List(enumType)), Nil)
val enumclazz = atSpan(start, nameOffset) {
TypeDef(name,
makeTemplate(superclazz :: interfaces, body, List(), true)).withMods(mods | Flags.JavaEnum)
Expand Down
27 changes: 12 additions & 15 deletions compiler/src/dotty/tools/dotc/transform/CompleteJavaEnums.scala
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,17 @@ class CompleteJavaEnums extends MiniPhase with InfoTransformer { thisPhase =>
// Because it adds additional parameters to some constructors

def transformInfo(tp: Type, sym: Symbol)(implicit ctx: Context): Type =
if (sym.isConstructor && derivesFromJEnum(sym.owner)) addConstrParams(sym.info)
if (sym.isConstructor && (
sym == defn.JavaEnumClass.primaryConstructor ||
derivesFromJavaEnum(sym.owner)))
addConstrParams(sym.info)
else tp

/** Is `sym` a Scala enum class that derives (directly) from `java.lang.Enum`?
*/
private def derivesFromJEnum(sym: Symbol)(implicit ctx: Context) =
private def derivesFromJavaEnum(sym: Symbol)(implicit ctx: Context) =
sym.is(Enum, butNot = Case) &&
sym.info.parents.exists(p => p.typeSymbol == defn.JEnumClass)
sym.info.parents.exists(p => p.typeSymbol == defn.JavaEnumClass)

/** Add constructor parameters `$name: String` and `$ordinal: Int` to the end of
* the last parameter list of (method- or poly-) type `tp`.
Expand Down Expand Up @@ -98,10 +101,10 @@ class CompleteJavaEnums extends MiniPhase with InfoTransformer { thisPhase =>
*/
override def transformDefDef(tree: DefDef)(implicit ctx: Context): DefDef = {
val sym = tree.symbol
if (sym.isConstructor && derivesFromJEnum(sym.owner))
if (sym.isConstructor && derivesFromJavaEnum(sym.owner))
cpy.DefDef(tree)(
vparamss = tree.vparamss.init :+ (tree.vparamss.last ++ addedParams(sym, Param)))
else if (sym.name == nme.DOLLAR_NEW && derivesFromJEnum(sym.owner.linkedClass)) {
else if (sym.name == nme.DOLLAR_NEW && derivesFromJavaEnum(sym.owner.linkedClass)) {
val Block((tdef @ TypeDef(tpnme.ANON_CLASS, templ: Template)) :: Nil, call) = tree.rhs
val args = tree.vparamss.last.takeRight(2).map(param => ref(param.symbol)).reverse
val templ1 = cpy.Template(templ)(
Expand All @@ -113,8 +116,7 @@ class CompleteJavaEnums extends MiniPhase with InfoTransformer { thisPhase =>
}

/** 1. If this is an enum class, add $name and $ordinal parameters to its
* parameter accessors and pass them on to the java.lang.Enum constructor,
* replacing the dummy arguments that were passed before.
* parameter accessors and pass them on to the java.lang.Enum constructor.
*
* 2. If this is an anonymous class that implement a value enum case,
* pass $name and $ordinal parameters to the enum superclass. The class
Expand All @@ -135,20 +137,15 @@ class CompleteJavaEnums extends MiniPhase with InfoTransformer { thisPhase =>
*/
override def transformTemplate(templ: Template)(implicit ctx: Context): Template = {
val cls = templ.symbol.owner
if (derivesFromJEnum(cls)) {
if (derivesFromJavaEnum(cls)) {
val (params, rest) = decomposeTemplateBody(templ.body)
val addedDefs = addedParams(cls, ParamAccessor)
val addedSyms = addedDefs.map(_.symbol.entered)
val parents1 = templ.parents.map {
case app @ Apply(fn, _) if fn.symbol.owner == defn.JEnumClass =>
cpy.Apply(app)(fn, addedSyms.map(ref))
case p => p
}
cpy.Template(templ)(
parents = parents1,
parents = addEnumConstrArgs(defn.JavaEnumClass, templ.parents, addedSyms.map(ref)),
body = params ++ addedDefs ++ rest)
}
else if (cls.isAnonymousClass && cls.owner.is(EnumCase) && derivesFromJEnum(cls.owner.owner.linkedClass)) {
else if (cls.isAnonymousClass && cls.owner.is(EnumCase) && derivesFromJavaEnum(cls.owner.owner.linkedClass)) {
def rhsOf(name: TermName) =
templ.body.collect {
case mdef: DefDef if mdef.name == name => mdef.rhs
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1071,8 +1071,8 @@ trait Checking {
(cls.owner.flagsUNSAFE.is(Case) || cls.owner.name == nme.DOLLAR_NEW)
if (!isEnumAnonCls) {
if (cdef.mods.isEnumCase) {
if (cls.derivesFrom(defn.JEnumClass))
ctx.error(em"parameterized case is not allowed in an enum that extends java.lang.Enum", cdef.sourcePos)
if (cls.derivesFrom(defn.JavaEnumClass))
ctx.error(em"paramerized case is not allowed in an enum that extends java.lang.Enum", cdef.sourcePos)
}
else if (cls.is(Case) || firstParent.is(Enum))
// Since enums are classes and Namer checks that classes don't extend multiple classes, we only check the class
Expand Down
9 changes: 0 additions & 9 deletions library/src/scala/compat/JEnum.scala

This file was deleted.

2 changes: 1 addition & 1 deletion tests/neg/enum-constrs.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

enum E[+T] extends compat.JEnum[E[_]] {
enum E[+T] extends java.lang.Enum[E[_]] {
case S1, S2
case C() extends E[Int] // error: parameterized case is not allowed
}
6 changes: 3 additions & 3 deletions tests/run/enum-constrs.scala
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
enum Color extends compat.JEnum[Color] {
enum Color extends java.lang.Enum[Color] {
case Red, Green, Blue
}

enum E[+T] extends compat.JEnum[E[_]] {
enum E[+T] extends java.lang.Enum[E[_]] {
case S1, S2
case C extends E[Int]
}

enum Vehicle(wheels: Int) extends compat.JEnum[Vehicle] {
enum Vehicle(wheels: Int) extends java.lang.Enum[Vehicle] {
case Bike extends Vehicle(2)
case Car extends Vehicle(4)
}
Expand Down