From ab257d62aec6a417c45fa7a7ea873358db0b07d1 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Fri, 18 Sep 2020 19:39:38 +0200 Subject: [PATCH] Don't use a special representation for Java enum values 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. --- .../tools/backend/jvm/BCodeBodyBuilder.scala | 18 +------------ .../tools/backend/jvm/BCodeHelpers.scala | 8 ++---- .../dotty/tools/backend/sjs/JSCodeGen.scala | 2 -- compiler/src/dotty/tools/dotc/ast/untpd.scala | 3 +++ .../src/dotty/tools/dotc/core/Constants.scala | 6 ----- .../dotc/core/classfile/ClassfileParser.scala | 13 +++------- .../tools/dotc/core/tasty/TreePickler.scala | 3 --- .../tools/dotc/core/tasty/TreeUnpickler.scala | 2 -- .../core/unpickleScala2/Scala2Unpickler.scala | 20 ++++++++++----- .../tools/dotc/printing/PlainPrinter.scala | 1 - tasty/src/dotty/tools/tasty/TastyFormat.scala | 25 ++++++++----------- 11 files changed, 35 insertions(+), 66 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala index 6d495c7d16f0..51d9e46fac3f 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala @@ -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. */ @@ -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") } } diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala index 34ec2b2bb747..ddcd15ba8571 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala @@ -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) @@ -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 { diff --git a/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala b/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala index cd7b1ba1ce26..038380e8bda9 100644 --- a/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala +++ b/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala @@ -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) => diff --git a/compiler/src/dotty/tools/dotc/ast/untpd.scala b/compiler/src/dotty/tools/dotc/ast/untpd.scala index 5e87ef6aeeed..3fd1210f61ad 100644 --- a/compiler/src/dotty/tools/dotc/ast/untpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/untpd.scala @@ -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())) diff --git a/compiler/src/dotty/tools/dotc/core/Constants.scala b/compiler/src/dotty/tools/dotc/core/Constants.scala index 040c6d7b45fc..474dc0a8b508 100644 --- a/compiler/src/dotty/tools/dotc/core/Constants.scala +++ b/compiler/src/dotty/tools/dotc/core/Constants.scala @@ -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 @@ -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. @@ -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 @@ -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 { @@ -253,7 +248,6 @@ object Constants { case x: String => StringTag case x: Char => CharTag case x: Type => ClazzTag - case x: Symbol => EnumTag } ) diff --git a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala index f4c95001ceaa..8316c83dad1d 100644 --- a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala +++ b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala @@ -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() @@ -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]() diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala index 575f58e273bf..b7613c67b706 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala @@ -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 diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index b9d7977422c1..07579cab87b8 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -286,8 +286,6 @@ class TreeUnpickler(reader: TastyReader, Constant(null) case CLASSconst => Constant(readType()) - case ENUMconst => - Constant(readTermRef().termSymbol) } /** Read a type */ diff --git a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala index 956cd0380712..6bcd689a71a3 100644 --- a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala @@ -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() @@ -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 { @@ -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) } } @@ -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 @@ -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) @@ -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) diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index 58149b76b663..062ab54c3037 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -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)) } diff --git a/tasty/src/dotty/tools/tasty/TastyFormat.scala b/tasty/src/dotty/tools/tasty/TastyFormat.scala index 4d8e4354f420..a857d21ca101 100644 --- a/tasty/src/dotty/tools/tasty/TastyFormat.scala +++ b/tasty/src/dotty/tools/tasty/TastyFormat.scala @@ -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. @@ -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 */ @@ -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 @@ -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"