From 5f40270060add9b5b3b3187b4ec03357973e51ea Mon Sep 17 00:00:00 2001 From: bishabosha Date: Fri, 28 Aug 2020 21:57:20 +0200 Subject: [PATCH 1/2] enum values array is constructed from field references This patches a regression introduced in #9628 where defining an enum local to a method can cause an infinite loop at initialisation of its companion. Instead, we select enum values from "this" and not the companion, which avoids forcing initialisation of the companion. We then ascribe the values array with unchecked annotation to avoid complaints from the init checker. --- .../src/dotty/tools/dotc/ast/DesugarEnums.scala | 15 ++++++++------- tests/run/enums-thunk.scala | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala b/compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala index acea4610fdff..1f412d6b6085 100644 --- a/compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala +++ b/compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala @@ -104,7 +104,7 @@ object DesugarEnums { /** The following lists of definitions for an enum type E and known value cases e_0, ..., e_n: * - * private val $values = Array[E](e_0,...,e_n)(ClassTag[E](classOf[E])) + * private val $values = Array[E](e_0,...,e_n)(ClassTag[E](classOf[E])): @unchecked * def values = $values.clone * def valueOf($name: String) = $name match { * case "e_0" => e_0 @@ -117,9 +117,11 @@ object DesugarEnums { val rawEnumClassRef = rawRef(enumClass.typeRef) extension (tpe: NamedType) def ofRawEnum = AppliedTypeTree(ref(tpe), rawEnumClassRef) - val lazyFlagOpt = if enumCompanion.owner.isStatic then EmptyFlags else Lazy - val privateValuesDef = ValDef(nme.DOLLAR_VALUES, TypeTree(), ArrayLiteral(enumValues, rawEnumClassRef)) - .withFlags(Private | Synthetic | lazyFlagOpt) + val privateValuesDef = + val uncheckedValues = + Annotated(ArrayLiteral(enumValues, rawEnumClassRef), New(ref(defn.UncheckedAnnot.typeRef))) + ValDef(nme.DOLLAR_VALUES, TypeTree(), uncheckedValues) + .withFlags(Private | Synthetic) val valuesDef = DefDef(nme.values, Nil, Nil, defn.ArrayType.ofRawEnum, valuesDot(nme.clone_)) @@ -170,7 +172,6 @@ object DesugarEnums { * def ordinal = _$ordinal // if `E` does not derive from `java.lang.Enum` * def enumLabel = $name // if `E` does not derive from `java.lang.Enum` * def enumLabel = this.name // if `E` derives from `java.lang.Enum` - * $values.register(this) * } */ private def enumValueCreator(using Context) = { @@ -307,8 +308,8 @@ object DesugarEnums { case name: TermName => (ordinal, name) :: seenCases case _ => seenCases if definesLookups then - val companionRef = ref(enumCompanion.termRef) - val cachedValues = cases.reverse.map((i, name) => (i, Select(companionRef, name))) + val thisRef = This(EmptyTypeIdent) + val cachedValues = cases.reverse.map((i, name) => (i, Select(thisRef, name))) (ordinal, enumLookupMethods(EnumConstraints(minKind, maxKind, cachedValues))) else ctx.tree.pushAttachment(EnumCaseCount, (ordinal + 1, minKind, maxKind, cases)) diff --git a/tests/run/enums-thunk.scala b/tests/run/enums-thunk.scala index 8ca9dde31e79..446d8b51d8f9 100644 --- a/tests/run/enums-thunk.scala +++ b/tests/run/enums-thunk.scala @@ -20,8 +20,22 @@ object Outer2 { } } +object Outer3 { + def thunk() = { + enum E { case C1 } + E.C1 + } + def thunk2() = { + enum E { case C2 } + E.values + } +} + + @main def Test = assert(Outer().thunk().toString == "A1") assert(Outer().thunk2()(0).toString == "A2") assert(Outer2.thunk().toString == "B1") assert(Outer2.thunk2()(0).toString == "B2") + assert(Outer3.thunk().toString == "C1") + assert(Outer3.thunk2()(0).toString == "C2") From bdea97a93df1a245aafec6b7c200551ade15ae4a Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Wed, 2 Sep 2020 11:14:39 +0200 Subject: [PATCH 2/2] add comment about unchecked annotation --- compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala b/compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala index 1f412d6b6085..4ed857678b26 100644 --- a/compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala +++ b/compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala @@ -104,12 +104,12 @@ object DesugarEnums { /** The following lists of definitions for an enum type E and known value cases e_0, ..., e_n: * - * private val $values = Array[E](e_0,...,e_n)(ClassTag[E](classOf[E])): @unchecked + * private val $values = Array[E](this.e_0,...,this.e_n)(ClassTag[E](classOf[E])): @unchecked * def values = $values.clone * def valueOf($name: String) = $name match { - * case "e_0" => e_0 + * case "e_0" => this.e_0 * ... - * case "e_n" => e_n + * case "e_n" => this.e_n * case _ => throw new IllegalArgumentException("case not found: " + $name) * } */ @@ -119,6 +119,12 @@ object DesugarEnums { val privateValuesDef = val uncheckedValues = + // Here we use an unchecked annotation to silence warnings from the init checker. Without it, we get a warning + // that simple enum cases are promoting this from warm to initialised. This is because we are populating the + // array by selecting enum values from `this`, a value under construction. + // Singleton enum values always construct a new anonymous class, which will not be checked by the init-checker, + // so this warning will always persist even if the implementation of the anonymous class is safe. + // TODO: remove @unchecked after https://github.com/lampepfl/dotty-feature-requests/issues/135 is resolved. Annotated(ArrayLiteral(enumValues, rawEnumClassRef), New(ref(defn.UncheckedAnnot.typeRef))) ValDef(nme.DOLLAR_VALUES, TypeTree(), uncheckedValues) .withFlags(Private | Synthetic)