Merge pull request #10493 from dotty-staging/release-fence

Fix #9424: Add `releaseFence()` call when mixing in a trait `val`.

Author: smarter

compiler/src/dotty/tools/dotc/core/StdNames.scala

@@ -562,6 +562,7 @@ object StdNames {
     val reflect: N              = "reflect"
     val reflectiveSelectable: N = "reflectiveSelectable"
     val reify : N               = "reify"
+    val releaseFence : N        = "releaseFence"
     val rootMirror : N          = "rootMirror"
     val run: N                  = "run"
     val runOrElse: N            = "runOrElse"

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

@@ -16,9 +16,16 @@ import NameKinds.TraitSetterName
 import NameOps._
 import Flags._
 import Decorators._
+import StdNames.nme
+
+import util.Store
 
 object Memoize {
   val name: String = "memoize"
+
+  private final class MyState {
+    val classesThatNeedReleaseFence = new util.HashSet[Symbol]
+  }
 }
 
 /** Provides the implementations of all getters and setters, introducing
@@ -37,10 +44,17 @@ object Memoize {
  *      --> <accessor> <mods> def x_=(y: T): Unit = x = y
  */
 class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase =>
+  import Memoize.MyState
   import ast.tpd._
 
   override def phaseName: String = Memoize.name
 
+  private var MyState: Store.Location[MyState] = _
+  private def myState(using Context): MyState = ctx.store(MyState)
+
+  override def initContext(ctx: FreshContext): Unit =
+    MyState = ctx.addLocation[MyState]()
+
   /* Makes sure that, after getters and constructors gen, there doesn't
    * exist non-deferred definitions that are not implemented. */
   override def checkPostCondition(tree: Tree)(using Context): Unit = {
@@ -69,6 +83,17 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase =>
    */
   override def runsAfter: Set[String] = Set(Mixin.name)
 
+  override def prepareForUnit(tree: Tree)(using Context): Context =
+    ctx.fresh.updateStore(MyState, new MyState())
+
+  override def transformTemplate(tree: Template)(using Context): Tree =
+    val cls = ctx.owner.asClass
+    if myState.classesThatNeedReleaseFence.contains(cls) then
+      val releaseFenceCall = ref(defn.staticsMethodRef(nme.releaseFence)).appliedToNone
+      cpy.Template(tree)(tree.constr, tree.parents, Nil, tree.self, tree.body :+ releaseFenceCall)
+    else
+      tree
+
   override def transformDefDef(tree: DefDef)(using Context): Tree = {
     val sym = tree.symbol
 
@@ -154,7 +179,11 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase =>
           // See tests/run/traitValOverriddenByParamAccessor.scala
           tree
         else
-          field.setFlag(Mutable) // Necessary for vals mixed in from traits
+          if !field.is(Mutable) then
+            // This is a val mixed in from a trait.
+            // We make it mutable, and mark the class as needing a releaseFence() in the constructor
+            field.setFlag(Mutable)
+            myState.classesThatNeedReleaseFence += sym.owner
           val initializer =
             if (isErasableBottomField(field, tree.vparamss.head.head.tpt.tpe.classSymbol)) Literal(Constant(()))
             else Assign(ref(field), adaptToField(field, ref(tree.vparamss.head.head.symbol)))

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

@@ -561,6 +561,93 @@ class TestBCode extends DottyBytecodeTest {
     }
   }
 
+  /* Test that vals in traits cause appropriate `releaseFence()` calls to be emitted. */
+
+  private def checkReleaseFence(releaseFenceExpected: Boolean, outputClassName: String, source: String): Unit = {
+    checkBCode(source) { dir =>
+      val clsIn = dir.lookupName(outputClassName, directory = false)
+      val clsNode = loadClassNode(clsIn.input)
+      val method = getMethod(clsNode, "<init>")
+
+      val hasReleaseFence = instructionsFromMethod(method).exists {
+        case Invoke(_, _, "releaseFence", _, _) => true
+        case _ => false
+      }
+
+      assertEquals(source, releaseFenceExpected, hasReleaseFence)
+    }
+  }
+
+  @Test def testInsertReleaseFence(): Unit = {
+    // An empty trait does not cause a releaseFence.
+    checkReleaseFence(false, "Bar.class",
+      """trait Foo {
+        |}
+        |class Bar extends Foo
+      """.stripMargin)
+
+    // A val in a class does not cause a releaseFence.
+    checkReleaseFence(false, "Bar.class",
+      """trait Foo {
+        |}
+        |class Bar extends Foo {
+        |  val x: Int = 5
+        |}
+      """.stripMargin)
+
+    // A val in a trait causes a releaseFence.
+    checkReleaseFence(true, "Bar.class",
+      """trait Foo {
+        |  val x: Int = 5
+        |}
+        |class Bar extends Foo
+      """.stripMargin)
+
+    // The presence of a var in the trait does not invalidate the need for a releaseFence.
+    // Also, indirect mixin.
+    checkReleaseFence(true, "Bar.class",
+      """trait Parent {
+        |  val x: Int = 5
+        |  var y: Int = 6
+        |}
+        |trait Foo extends Parent
+        |class Bar extends Foo
+      """.stripMargin)
+
+    // The presence of a var in the class does not invalidate the need for a releaseFence.
+    checkReleaseFence(true, "Bar.class",
+      """trait Foo {
+        |  val x: Int = 5
+        |}
+        |class Bar extends Foo {
+        |  var y: Int = 6
+        |}
+      """.stripMargin)
+
+    // When inheriting trait vals through a superclass, no releaseFence is inserted.
+    checkReleaseFence(false, "Bar.class",
+      """trait Parent {
+        |  val x: Int = 5
+        |  var y: Int = 6
+        |}
+        |class Foo extends Parent // releaseFence in Foo, but not in Bar
+        |class Bar extends Foo
+      """.stripMargin)
+
+    // Various other stuff that do not cause a releaseFence.
+    checkReleaseFence(false, "Bar.class",
+      """trait Foo {
+        |  var w: Int = 1
+        |  final val x = 2
+        |  def y: Int = 3
+        |  lazy val z: Int = 4
+        |
+        |  def add(a: Int, b: Int): Int = a + b
+        |}
+        |class Bar extends Foo
+      """.stripMargin)
+  }
+
   /* Test that objects compile to *final* classes. */
 
   private def checkFinalClass(outputClassName: String, source: String) = {