Skip to content
/ scala3 Public
forked from scala/scala3

Commit

Permalink
Don't use a special representation for Java enum values
Browse files Browse the repository at this point in the history
In Java, annotation arguments must be constants, so enum values are
treated specially. In Scala, annotation arguments can be whatever tree
we want, so we don't really need to represent them as
`Literal(Constant(enumValueSymbol))` and can just use a `TermRef` to the
enum value instead.

This commit implements this change, this has several advantages compared
to the status quo:
- The handling of Java enum values and Scala enum values is now less
  different.
- We can drop the handling of symbols in `Constant`
- ... and therefore remove one tag from Tasty.
- In turn, this means we don't have to worry about how to expose
  this in tasty-reflect.

This commit breaks the Tasty format and therefore bump its major version
to 24.
  • Loading branch information
smarter committed Oct 1, 2020
1 parent 94970a0 commit ab257d6
Show file tree
Hide file tree
Showing 11 changed files with 35 additions and 66 deletions.
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

0 comments on commit ab257d6

Please sign in to comment.