Skip to content

Don't use a special representation for Java enum values #9930

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
Oct 2, 2020
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
18 changes: 1 addition & 17 deletions compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
// ---------------- emitting constant values ----------------

/*
* For const.tag in {ClazzTag, EnumTag}
* For ClazzTag:
* must-single-thread
* Otherwise it's safe to call from multiple threads.
*/
Expand Down Expand Up @@ -527,22 +527,6 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
else
mnode.visitLdcInsn(tp.toASMType)

case EnumTag =>
val sym = const.symbolValue
val ownerName = internalName(sym.owner)
val fieldName = sym.javaSimpleName
val underlying = sym.info match { // TODO: Is this actually necessary? Could it be replaced by a call to widen?
case t: TypeProxy => t.underlying
case t => t
}
val fieldDesc = toTypeKind(underlying).descriptor
mnode.visitFieldInsn(
asm.Opcodes.GETSTATIC,
ownerName,
fieldName,
fieldDesc
)

case _ => abort(s"Unknown constant value: $const")
}
}
Expand Down
8 changes: 2 additions & 6 deletions compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -377,14 +377,10 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
assert(const.value != null, const) // TODO this invariant isn't documented in `case class Constant`
av.visit(name, const.stringValue) // `stringValue` special-cases null, but that execution path isn't exercised for a const with StringTag
case ClazzTag => av.visit(name, typeToTypeKind(TypeErasure.erasure(const.typeValue))(bcodeStore)(innerClasesStore).toASMType)
case EnumTag =>
val edesc = innerClasesStore.typeDescriptor(const.tpe) // the class descriptor of the enumeration class.
val evalue = const.symbolValue.javaSimpleName // value the actual enumeration value.
av.visitEnum(name, edesc, evalue)
}
case Ident(nme.WILDCARD) =>
// An underscore argument indicates that we want to use the default value for this parameter, so do not emit anything
case t: tpd.RefTree if t.symbol.denot.owner.isAllOf(JavaEnumTrait) =>
case t: tpd.RefTree if t.symbol.owner.linkedClass.isAllOf(JavaEnumTrait) =>
val edesc = innerClasesStore.typeDescriptor(t.tpe) // the class descriptor of the enumeration class.
val evalue = t.symbol.javaSimpleName // value the actual enumeration value.
av.visitEnum(name, edesc, evalue)
Expand Down Expand Up @@ -454,7 +450,7 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {

private def retentionPolicyOf(annot: Annotation): Symbol =
annot.tree.tpe.typeSymbol.getAnnotation(AnnotationRetentionAttr).
flatMap(_.argumentConstant(0).map(_.symbolValue)).getOrElse(AnnotationRetentionClassAttr)
flatMap(_.argument(0).map(_.tpe.termSymbol)).getOrElse(AnnotationRetentionClassAttr)

private def assocsFromApply(tree: Tree): List[(Name, Tree)] = {
tree match {
Expand Down
2 changes: 0 additions & 2 deletions compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1223,8 +1223,6 @@ class JSCodeGen()(using genCtx: Context) {
js.Null()
case ClazzTag =>
genClassConstant(value.typeValue)
case EnumTag =>
genLoadStaticField(value.symbolValue)
}

case Block(stats, expr) =>
Expand Down
3 changes: 3 additions & 0 deletions compiler/src/dotty/tools/dotc/ast/untpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,9 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
def ref(tp: NamedType)(using Context): Tree =
TypedSplice(tpd.ref(tp))

def ref(sym: Symbol)(using Context): Tree =
TypedSplice(tpd.ref(sym))

def rawRef(tp: NamedType)(using Context): Tree =
if tp.typeParams.isEmpty then ref(tp)
else AppliedTypeTree(ref(tp), tp.typeParams.map(_ => WildcardTypeBoundsTree()))
Expand Down
6 changes: 0 additions & 6 deletions compiler/src/dotty/tools/dotc/core/Constants.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ object Constants {
final val StringTag = 10
final val NullTag = 11
final val ClazzTag = 12
// For supporting java enumerations inside java annotations (see ClassfileParser)
final val EnumTag = 13

class Constant(val value: Any, val tag: Int) extends printing.Showable with Product1[Any] {
import java.lang.Double.doubleToRawLongBits
Expand Down Expand Up @@ -50,7 +48,6 @@ object Constants {
case StringTag => defn.StringType
case NullTag => defn.NullType
case ClazzTag => defn.ClassType(typeValue)
case EnumTag => defn.EnumType(symbolValue)
}

/** We need the equals method to take account of tags as well as values.
Expand Down Expand Up @@ -190,7 +187,6 @@ object Constants {
def toText(printer: Printer): Text = printer.toText(this)

def typeValue: Type = value.asInstanceOf[Type]
def symbolValue: Symbol = value.asInstanceOf[Symbol]

/**
* Consider two `NaN`s to be identical, despite non-equality
Expand Down Expand Up @@ -237,7 +233,6 @@ object Constants {
def apply(x: String): Constant = new Constant(x, StringTag)
def apply(x: Char): Constant = new Constant(x, CharTag)
def apply(x: Type): Constant = new Constant(x, ClazzTag)
def apply(x: Symbol): Constant = new Constant(x, EnumTag)
def apply(value: Any): Constant =
new Constant(value,
value match {
Expand All @@ -253,7 +248,6 @@ object Constants {
case x: String => StringTag
case x: Char => CharTag
case x: Type => ClazzTag
case x: Symbol => EnumTag
}
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,6 @@ class ClassfileParser(

val isVarargs = denot.is(Flags.Method) && (jflags & JAVA_ACC_VARARGS) != 0
denot.info = pool.getType(in.nextChar, isVarargs)
if (isEnum) denot.info = ConstantType(Constant(sym))
if (isConstructor) normalizeConstructorParams()
denot.info = translateTempPoly(parseAttributes(sym, denot.info, isVarargs))
if (isConstructor) normalizeConstructorInfo()
Expand Down Expand Up @@ -525,17 +524,13 @@ class ClassfileParser(
case CLASS_TAG =>
if (skip) None else Some(lit(Constant(pool.getType(index))))
case ENUM_TAG =>
val t = pool.getType(index)
val n = pool.getName(in.nextChar)
val module = t.typeSymbol.companionModule
val s = module.info.decls.lookup(n)
val enumClassTp = pool.getType(index)
val enumCaseName = pool.getName(in.nextChar)
if (skip)
None
else if (s != NoSymbol)
Some(lit(Constant(s)))
else {
report.warning(s"""While parsing annotations in ${in.file}, could not find $n in enum $module.\nThis is likely due to an implementation restriction: an annotation argument cannot refer to a member of the annotated class (SI-7014).""")
None
val enumModuleClass = enumClassTp.classSymbol.companionModule
Some(Select(ref(enumModuleClass), enumCaseName))
}
case ARRAY_TAG =>
val arr = new ArrayBuffer[Tree]()
Expand Down
3 changes: 0 additions & 3 deletions compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,6 @@ class TreePickler(pickler: TastyPickler) {
case ClazzTag =>
writeByte(CLASSconst)
pickleType(c.typeValue)
case EnumTag =>
writeByte(ENUMconst)
pickleType(c.symbolValue.termRef)
}

def pickleVariances(tp: Type)(using Context): Unit = tp match
Expand Down
2 changes: 0 additions & 2 deletions compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,6 @@ class TreeUnpickler(reader: TastyReader,
Constant(null)
case CLASSconst =>
Constant(readType())
case ENUMconst =>
Constant(readTermRef().termSymbol)
}

/** Read a type */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,9 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
val supertpe = readTypeRef()
SuperType(thistpe, supertpe)
case CONSTANTtpe =>
ConstantType(readConstantRef())
readConstantRef() match
case c: Constant => ConstantType(c)
case tp: TermRef => tp
case TYPEREFtpe =>
var pre = readPrefix()
val sym = readSymbolRef()
Expand Down Expand Up @@ -822,7 +824,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
errorBadSignature("bad type tag: " + tag)

/** Read a constant */
protected def readConstant()(using Context): Constant = {
protected def readConstant()(using Context): Constant | TermRef = {
val tag = readByte().toInt
val len = readNat()
(tag: @switch) match {
Expand All @@ -838,7 +840,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
case LITERALstring => Constant(readNameRef().toString)
case LITERALnull => Constant(null)
case LITERALclass => Constant(readTypeRef())
case LITERALenum => Constant(readSymbolRef())
case LITERALenum => readSymbolRef().termRef
case _ => noSuchConstantTag(tag, len)
}
}
Expand Down Expand Up @@ -881,7 +883,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas

protected def readNameRef()(using Context): Name = at(readNat(), () => readName())
protected def readTypeRef()(using Context): Type = at(readNat(), () => readType()) // after the NMT_TRANSITION period, we can leave off the () => ... ()
protected def readConstantRef()(using Context): Constant = at(readNat(), () => readConstant())
protected def readConstantRef()(using Context): Constant | TermRef = at(readNat(), () => readConstant())

protected def readTypeNameRef()(using Context): TypeName = readNameRef().toTypeName
protected def readTermNameRef()(using Context): TermName = readNameRef().toTermName
Expand All @@ -896,7 +898,11 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
*/
protected def readAnnotArg(i: Int)(using Context): Tree = bytes(index(i)) match {
case TREE => at(i, () => readTree())
case _ => Literal(at(i, () => readConstant()))
case _ => at(i, () =>
readConstant() match
case c: Constant => Literal(c)
case tp: TermRef => ref(tp)
)
}

/** Read a ClassfileAnnotArg (argument to a classfile annotation)
Expand Down Expand Up @@ -1208,7 +1214,9 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
Ident(symbol.namedType)

case LITERALtree =>
Literal(readConstantRef())
readConstantRef() match
case c: Constant => Literal(c)
case tp: TermRef => ref(tp)

case TYPEtree =>
TypeTree(tpe)
Expand Down
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,6 @@ class PlainPrinter(_ctx: Context) extends Printer {
case ClazzTag => "classOf[" ~ toText(const.typeValue) ~ "]"
case CharTag => literalText(s"'${escapedChar(const.charValue)}'")
case LongTag => literalText(const.longValue.toString + "L")
case EnumTag => literalText(const.symbolValue.name.toString)
case _ => literalText(String.valueOf(const.value))
}

Expand Down
25 changes: 11 additions & 14 deletions tasty/src/dotty/tools/tasty/TastyFormat.scala
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ Standard-Section: "ASTs" TopLevelStat*
STRINGconst NameRef -- A string literal
NULLconst -- null
CLASSconst Type -- classOf[Type]
ENUMconst Path -- An enum constant

Type = Path -- Paths represent both types and terms
TYPEREFdirect sym_ASTRef -- A reference to a local symbol (without a prefix). Reference is to definition node of symbol.
Expand Down Expand Up @@ -254,7 +253,7 @@ Standard Section: "Comments" Comment*
object TastyFormat {

final val header: Array[Int] = Array(0x5C, 0xA1, 0xAB, 0x1F)
val MajorVersion: Int = 23
val MajorVersion: Int = 24
val MinorVersion: Int = 0

/** Tags used to serialize names, should update [[nameTagToString]] if a new constant is added */
Expand Down Expand Up @@ -387,17 +386,16 @@ object TastyFormat {
final val THIS = 80
final val QUALTHIS = 81
final val CLASSconst = 82
final val ENUMconst = 83
final val BYNAMEtype = 84
final val BYNAMEtpt = 85
final val NEW = 86
final val THROW = 87
final val IMPLICITarg = 88
final val PRIVATEqualified = 89
final val PROTECTEDqualified = 90
final val RECtype = 91
final val SINGLETONtpt = 92
final val BOUNDED = 93
final val BYNAMEtype = 83
final val BYNAMEtpt = 84
final val NEW = 85
final val THROW = 86
final val IMPLICITarg = 87
final val PRIVATEqualified = 88
final val PROTECTEDqualified = 89
final val RECtype = 90
final val SINGLETONtpt = 91
final val BOUNDED = 92

// Cat. 4: tag Nat AST

Expand Down Expand Up @@ -651,7 +649,6 @@ object TastyFormat {
case QUALTHIS => "QUALTHIS"
case SUPER => "SUPER"
case CLASSconst => "CLASSconst"
case ENUMconst => "ENUMconst"
case SINGLETONtpt => "SINGLETONtpt"
case SUPERtype => "SUPERtype"
case TERMREFin => "TERMREFin"
Expand Down