enum values array is constructed from field references #9677
Conversation
This patches a regression introduced in scala#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.
| val uncheckedValues = | ||
| Annotated(ArrayLiteral(enumValues, rawEnumClassRef), New(ref(defn.UncheckedAnnot.typeRef))) | ||
| ValDef(nme.DOLLAR_VALUES, TypeTree(), uncheckedValues) | ||
| .withFlags(Private | Synthetic) |
There was a problem hiding this comment.
The lazyFlagOpt disappeared. Was this a mistake during conflict resolution?
There was a problem hiding this comment.
no, it was removed because it was only required when the enum values were explicitly selected from the companion, and the companion was a local lazy val. Now they are selected from this which doesn't force the companion to be initialised.
| @@ -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 | |||
There was a problem hiding this comment.
Please add a comment explaining the interaction with the init checker. Is this a known limitation of the init checker? Is it something that can be fixed and if so is there an issue for it?
There was a problem hiding this comment.
The issue is that the init checker will not try to prove that new E {...} is safe for performance reasons, so all enum cases that call $new creator method can't be checked. The init checker however knows that selecting from E companion object must be safe because it is already constructed, so we can either select from this which means $values does not have to be lazy, or we have to select from E which is under construction so it has to be lazy.
There was a problem hiding this comment.
we also have this comment #9665 (comment)
There was a problem hiding this comment.
OK, as long as this is all properly documented and referenced in the code, I'm happy :).
There was a problem hiding this comment.
In the long run, we might introduce type annotations and/or eagerly check small classes, both could be helpful to avoid @unchecked in such cases. Initially, we want to keep the checker stupid to see how it works in the wild and make sure the core algorithm is reliable.
There was a problem hiding this comment.
Maybe an issue should be opened for that (either here or on dotty-feature-requests) so that Jamie can link to it in the code?
There was a problem hiding this comment.
bishabosha
force-pushed
the
fix-enum-values
branch
from
f1e3fd4 to
bdea97a
Compare
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.