Merge pull request #9261 from dotty-staging/keep-inlined-final-vals-in-generated-code

Keep inlined final vals in generated code

Author: nicolasstucki

compiler/src/dotty/tools/dotc/transform/Memoize.scala

@@ -114,7 +114,9 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase =>
         EmptyTree
       }
 
-    if (sym.is(Accessor, butNot = NoFieldNeeded)) {
+    val constantFinalVal = sym.isAllOf(Accessor | Final, butNot = Mutable) && tree.rhs.isInstanceOf[Literal]
+
+    if (sym.is(Accessor, butNot = NoFieldNeeded) && !constantFinalVal) {
       val field = sym.field.orElse(newField).asTerm
 
       def adaptToField(tree: Tree): Tree =

compiler/src/dotty/tools/dotc/typer/Checking.scala

@@ -869,11 +869,9 @@ trait Checking {
   /** Check that `tree` can be right hand-side or argument to `inline` value or parameter. */
   def checkInlineConformant(tpt: Tree, tree: Tree, sym: Symbol)(using Context): Unit = {
     if sym.is(Inline, butNot = DeferredOrTermParamOrAccessor) && !ctx.erasedTypes && !Inliner.inInlineMethod then
-      // final vals can be marked inline even if they're not pure, see Typer#patchFinalVals
-      val purityLevel = if (sym.is(Final)) Idempotent else Pure
       tpt.tpe.widenTermRefExpr.dealias.normalized match
         case tp: ConstantType =>
-          if !(exprPurity(tree) >= purityLevel) then
+          if !(exprPurity(tree) >= Pure) then
             ctx.error(em"inline value must be pure", tree.sourcePos)
         case _ =>
           val pos = if tpt.span.isZeroExtent then tree.sourcePos else tpt.sourcePos

compiler/src/dotty/tools/dotc/typer/Namer.scala

@@ -1464,6 +1464,7 @@ class Namer { typer: Typer =>
 
       // println(s"final inherited for $sym: ${inherited.toString}") !!!
       // println(s"owner = ${sym.owner}, decls = ${sym.owner.info.decls.show}")
+      // TODO Scala 3.1: only check for inline vals (no final ones)
       def isInlineVal = sym.isOneOf(FinalOrInline, butNot = Method | Mutable)
 
       // Widen rhs type and eliminate `|' but keep ConstantTypes if

compiler/src/dotty/tools/dotc/typer/Typer.scala

@@ -1856,30 +1856,9 @@ class Typer extends Namer
     val vdef1 = assignType(cpy.ValDef(vdef)(name, tpt1, rhs1), sym)
     checkSignatureRepeatedParam(sym)
     checkInlineConformant(tpt1, rhs1, sym)
-    patchFinalVals(vdef1)
     vdef1.setDefTree
   }
 
-  /** Adds inline to final vals with idempotent rhs
-   *
-   *  duplicating scalac behavior: for final vals that have rhs as constant, we do not create a field
-   *  and instead return the value. This seemingly minor optimization has huge effect on initialization
-   *  order and the values that can be observed during superconstructor call
-   *
-   *  see remark about idempotency in TreeInfo#constToLiteral
-   */
-  private def patchFinalVals(vdef: ValDef)(using Context): Unit = {
-    def isFinalInlinableVal(sym: Symbol): Boolean =
-      sym.is(Final, butNot = Mutable) &&
-      isIdempotentExpr(vdef.rhs) /* &&
-      ctx.scala2Mode (stay compatible with Scala2 for now) */
-    val sym = vdef.symbol
-    sym.info match {
-      case info: ConstantType if isFinalInlinableVal(sym) && !ctx.settings.YnoInline.value => sym.setFlag(Inline)
-      case _ =>
-    }
-  }
-
   def typedDefDef(ddef: untpd.DefDef, sym: Symbol)(using Context): Tree = {
     if (!sym.info.exists) { // it's a discarded synthetic case class method, drop it
       assert(sym.is(Synthetic) && desugar.isRetractableCaseClassMethodName(sym.name))

compiler/test/dotty/tools/backend/jvm/InlineBytecodeTests.scala

@@ -345,7 +345,7 @@ class InlineBytecodeTests extends DottyBytecodeTest {
   }
 
     // Testing that a is not boxed
-    @Test def i4522 = {
+  @Test def i4522 = {
     val source = """class Foo {
                    |  def test: Int = {
                    |    var a = 10
@@ -480,7 +480,28 @@ class InlineBytecodeTests extends DottyBytecodeTest {
       val expected = List(Ldc(LDC, 5.0), Op(DRETURN))
       assert(instructions == expected,
         "`divide` was not properly inlined in `test`\n" + diffInstructions(instructions, expected))
+    }
+  }
+
+  @Test def finalVals = {
+    val source = """class Test:
+                   |  final val a = 1 // should be inlined but not erased
+                   |  inline val b = 2 // should be inlined and erased
+                   |  def test: Int = a + b
+                 """.stripMargin
+
+    checkBCode(source) { dir =>
+      val clsIn      = dir.lookupName("Test.class", directory = false).input
+      val clsNode    = loadClassNode(clsIn)
+
+      val fun = getMethod(clsNode, "test")
+      val instructions = instructionsFromMethod(fun)
+      val expected = List(Op(ICONST_3), Op(IRETURN))
+      assert(instructions == expected,
+        "`a and b were not properly inlined in `test`\n" + diffInstructions(instructions, expected))
 
+      val methods = clsNode.methods.asScala.toList.map(_.name)
+      assert(methods == List("<init>", "a", "test"), clsNode.methods.asScala.toList.map(_.name))
     }
   }
 

tests/run/erased-inline-vals.scala

@@ -41,7 +41,7 @@ class D:
   assert(classOf[B].getDeclaredFields.isEmpty)
 
   assert(classOf[C].getDeclaredMethods.size == 2)
-  assert(classOf[C].getDeclaredFields.size == 1)
+  assert(classOf[C].getDeclaredFields.isEmpty)
 
   assert(classOf[D].getDeclaredMethods.isEmpty)
   assert(classOf[D].getFields.isEmpty)