diff --git a/compiler/src/dotty/tools/dotc/transform/Constructors.scala b/compiler/src/dotty/tools/dotc/transform/Constructors.scala index 59b90ff7f084..4dd7205e4ee0 100644 --- a/compiler/src/dotty/tools/dotc/transform/Constructors.scala +++ b/compiler/src/dotty/tools/dotc/transform/Constructors.scala @@ -226,31 +226,39 @@ class Constructors extends MiniPhase with IdentityDenotTransformer { thisPhase = constrStats += intoConstr(stat, sym) } else dropped += sym - case stat @ DefDef(name, _, tpt, _) - if stat.symbol.isGetter && stat.symbol.owner.is(Trait) && !stat.symbol.is(Lazy) && !stat.symbol.isConstExprFinalVal => + case stat @ DefDef(name, _, tpt, _) if stat.symbol.isGetter && !stat.symbol.is(Lazy) => val sym = stat.symbol assert(isRetained(sym), sym) - if !stat.rhs.isEmpty && !isWildcardArg(stat.rhs) then - /* !!! Work around #9390 - * This should really just be `sym.setter`. However, if we do that, we'll miss - * setters for mixed in `private var`s. Even though the scope clearly contains the - * setter symbol with the correct Name structure (since the `find` finds it), - * `.decl(setterName)` used by `.setter` through `.accessorNamed` will *not* find it. - * Could it be that the hash table of the `Scope` is corrupted? - * We still try `sym.setter` first as an optimization, since it will work for all - * public vars in traits and all (public or private) vars in classes. - */ - val symSetter = - if sym.setter.exists then - sym.setter - else - val setterName = sym.asTerm.name.setterName - sym.owner.info.decls.find(d => d.is(Accessor) && d.name == setterName) - val setter = - if (symSetter.exists) symSetter - else sym.accessorNamed(Mixin.traitSetterName(sym.asTerm)) - constrStats += Apply(ref(setter), intoConstr(stat.rhs, sym).withSpan(stat.span) :: Nil) - clsStats += cpy.DefDef(stat)(rhs = EmptyTree) + if sym.isConstExprFinalVal then + if stat.rhs.isInstanceOf[Literal] then + clsStats += stat + else + constrStats += intoConstr(stat.rhs, sym) + clsStats += cpy.DefDef(stat)(rhs = Literal(sym.constExprFinalValConstantType.value).withSpan(stat.span)) + else if !sym.owner.is(Trait) then + clsStats += stat + else + if !stat.rhs.isEmpty && !isWildcardArg(stat.rhs) then + /* !!! Work around #9390 + * This should really just be `sym.setter`. However, if we do that, we'll miss + * setters for mixed in `private var`s. Even though the scope clearly contains the + * setter symbol with the correct Name structure (since the `find` finds it), + * `.decl(setterName)` used by `.setter` through `.accessorNamed` will *not* find it. + * Could it be that the hash table of the `Scope` is corrupted? + * We still try `sym.setter` first as an optimization, since it will work for all + * public vars in traits and all (public or private) vars in classes. + */ + val symSetter = + if sym.setter.exists then + sym.setter + else + val setterName = sym.asTerm.name.setterName + sym.owner.info.decls.find(d => d.is(Accessor) && d.name == setterName) + val setter = + if (symSetter.exists) symSetter + else sym.accessorNamed(Mixin.traitSetterName(sym.asTerm)) + constrStats += Apply(ref(setter), intoConstr(stat.rhs, sym).withSpan(stat.span) :: Nil) + clsStats += cpy.DefDef(stat)(rhs = EmptyTree) case DefDef(nme.CONSTRUCTOR, ((outerParam @ ValDef(nme.OUTER, _, _)) :: _) :: Nil, _, _) => clsStats += mapOuter(outerParam.symbol).transform(stat) case _: DefTree => diff --git a/compiler/src/dotty/tools/dotc/transform/Memoize.scala b/compiler/src/dotty/tools/dotc/transform/Memoize.scala index 1392d00011a2..03ac15b39ffe 100644 --- a/compiler/src/dotty/tools/dotc/transform/Memoize.scala +++ b/compiler/src/dotty/tools/dotc/transform/Memoize.scala @@ -20,8 +20,6 @@ import sjs.JSSymUtils._ import util.Store -import dotty.tools.backend.sjs.JSDefinitions.jsdefn - object Memoize { val name: String = "memoize" val description: String = "add private fields to getters and setters" @@ -130,32 +128,17 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase => } if sym.is(Accessor, butNot = NoFieldNeeded) then - /* Tests whether the semantics of Scala.js require a field for this symbol, irrespective of any - * optimization we think we can do. This is the case if one of the following is true: - * - it is a member of a JS type, since it needs to be visible as a JavaScript field - * - is is exported as static member of the companion class, since it needs to be visible as a JavaScript static field - * - it is exported to the top-level, since that can only be done as a true top-level variable, i.e., a field - */ - def sjsNeedsField: Boolean = - ctx.settings.scalajs.value && ( - sym.owner.isJSType - || sym.hasAnnotation(jsdefn.JSExportTopLevelAnnot) - || sym.hasAnnotation(jsdefn.JSExportStaticAnnot) - ) - def adaptToField(field: Symbol, tree: Tree): Tree = if (tree.isEmpty) tree else tree.ensureConforms(field.info.widen) def isErasableBottomField(field: Symbol, cls: Symbol): Boolean = !field.isVolatile && ((cls eq defn.NothingClass) || (cls eq defn.NullClass) || (cls eq defn.BoxedUnitClass)) - && !sjsNeedsField + && !sym.sjsNeedsField if sym.isGetter then - val constantFinalVal = - sym.isAllOf(Accessor | Final, butNot = Mutable) && tree.rhs.isInstanceOf[Literal] && !sjsNeedsField - if constantFinalVal then - // constant final vals do not need to be transformed at all, and do not need a field + if sym.isConstExprFinalVal then + // const-expr final vals do not need to be transformed at all, and do not need a field tree else val field = newField.asTerm diff --git a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala index feb841ba5c6c..c02a7d90cb8c 100644 --- a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala +++ b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala @@ -18,6 +18,8 @@ import Annotations.Annotation import Phases._ import ast.tpd.Literal +import dotty.tools.dotc.transform.sjs.JSSymUtils.sjsNeedsField + import scala.annotation.tailrec object SymUtils: @@ -259,9 +261,29 @@ object SymUtils: self.owner.info.decl(fieldName).suchThat(!_.is(Method)).symbol } + /** Is this symbol a constant expression final val? + * + * This is the case if all of the following are true: + * + * - it is a `final val`, + * - its result type is a `ConstantType`, and + * - it does not need an explicit field because of Scala.js semantics (see `JSSymUtils.sjsNeedsField`). + * + * Constant expression final vals do not need an explicit field to store + * their value. See the Memoize-Mixin-Constructors phase trio. + */ def isConstExprFinalVal(using Context): Boolean = atPhaseNoLater(erasurePhase) { - self.is(Final) && self.info.resultType.isInstanceOf[ConstantType] + self.is(Final, butNot = Mutable) && self.info.resultType.isInstanceOf[ConstantType] + } && !self.sjsNeedsField + + /** The `ConstantType` of a val known to be `isConstrExprFinalVal`. + * + * @pre `self.isConstantExprFinalVal` is true. + */ + def constExprFinalValConstantType(using Context): ConstantType = + atPhaseNoLater(erasurePhase) { + self.info.resultType.asInstanceOf[ConstantType] } def isField(using Context): Boolean = diff --git a/compiler/src/dotty/tools/dotc/transform/sjs/JSSymUtils.scala b/compiler/src/dotty/tools/dotc/transform/sjs/JSSymUtils.scala index 30eed76b18ec..115d41dd3d46 100644 --- a/compiler/src/dotty/tools/dotc/transform/sjs/JSSymUtils.scala +++ b/compiler/src/dotty/tools/dotc/transform/sjs/JSSymUtils.scala @@ -211,6 +211,23 @@ object JSSymUtils { } } } + + /** Tests whether the semantics of Scala.js require a field for this symbol, + * irrespective of any optimization we think we can do. + * + * This is the case if one of the following is true: + * + * - it is a member of a JS type, since it needs to be visible as a JavaScript field + * - is is exported as static member of the companion class, since it needs to be visible as a JavaScript static field + * - it is exported to the top-level, since that can only be done as a true top-level variable, i.e., a field + */ + def sjsNeedsField(using Context): Boolean = + ctx.settings.scalajs.value && ( + sym.owner.isJSType + || sym.hasAnnotation(jsdefn.JSExportTopLevelAnnot) + || sym.hasAnnotation(jsdefn.JSExportStaticAnnot) + ) + end sjsNeedsField } private object JSUnaryOpMethodName { diff --git a/tests/run/erased-inline-vals.scala b/tests/run/erased-inline-vals.scala index 00c9c8c168c7..c39a8295af5d 100644 --- a/tests/run/erased-inline-vals.scala +++ b/tests/run/erased-inline-vals.scala @@ -16,6 +16,27 @@ class D: inline def x: Int = 5 inline val y = 6 +object SideEffects: + val sideEffects = scala.collection.mutable.ListBuffer.empty[String] + +trait E: + final val a: 7 = + SideEffects.sideEffects += "E.a" + 7 + final val b = + SideEffects.sideEffects += "E.b" + 8 +end E + +class F extends E: + final val c: 9 = + SideEffects.sideEffects += "F.c" + 9 + final val d = + SideEffects.sideEffects += "F.d" + 10 +end F + @main def Test = val b: B = new B assert(b.x == 1) @@ -37,12 +58,24 @@ class D: assert(d.x == 5) assert(d.y == 6) + val f: F = new F + assert(SideEffects.sideEffects.toList == List("E.a", "E.b", "F.c", "F.d")) + assert(f.a == 7) + assert(f.b == 8) + assert(f.c == 9) + assert(f.d == 10) assert(classOf[B].getDeclaredMethods.size == 2) assert(classOf[B].getDeclaredFields.isEmpty) assert(classOf[C].getDeclaredMethods.size == 2) - assert(classOf[C].getDeclaredFields.isEmpty) + assert(classOf[C].getDeclaredFields.size == 1) // x, but not y assert(classOf[D].getDeclaredMethods.isEmpty) assert(classOf[D].getFields.isEmpty) + + assert(classOf[E].getDeclaredMethods.size == 5) + assert(classOf[E].getDeclaredFields.isEmpty) + + assert(classOf[F].getDeclaredMethods.size == 2) + assert(classOf[F].getDeclaredFields.isEmpty) diff --git a/tests/run/final-fields.check b/tests/run/final-fields.check index 3ebde7d7f735..af090f65a32a 100644 --- a/tests/run/final-fields.check +++ b/tests/run/final-fields.check @@ -2,6 +2,6 @@ T.f1 T.f2 T.f3 T.f4 -3 2 -3 -4 +3 2 3 -4 3 g diff --git a/tests/run/i17549.check b/tests/run/i17549.check new file mode 100644 index 000000000000..df4c241f5ffe --- /dev/null +++ b/tests/run/i17549.check @@ -0,0 +1,6 @@ +T.x +C.y +1 +2 +1 +2 diff --git a/tests/run/i17549.scala b/tests/run/i17549.scala new file mode 100644 index 000000000000..165e40512153 --- /dev/null +++ b/tests/run/i17549.scala @@ -0,0 +1,27 @@ +trait T: + final val x: 1 = + println("T.x") + 1 +end T + +trait U: + def x: Any + def y: Any + +class C extends T with U: + final val y: 2 = + println("C.y") + 2 +end C + +object Test: + def main(args: Array[String]): Unit = + val c = new C + println(c.x) + println(c.y) + + val u: U = c + println(u.x) + println(u.y) + end main +end Test