From f256aa85053cf099d2da84a8855a1d0c2738f26e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 25 Nov 2020 16:43:30 +0100 Subject: [PATCH] Fix #9424: Add `releaseFence()` call when mixing in a trait `val`. This is a forward port of relevant parts of the upstream PR https://github.com/scala/scala/pull/7028 --- .../src/dotty/tools/dotc/core/StdNames.scala | 1 + .../dotty/tools/dotc/transform/Memoize.scala | 31 ++++++- .../backend/jvm/DottyBytecodeTests.scala | 87 +++++++++++++++++++ 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala index 3368813d414f..3eeae9a2ffe9 100644 --- a/compiler/src/dotty/tools/dotc/core/StdNames.scala +++ b/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" diff --git a/compiler/src/dotty/tools/dotc/transform/Memoize.scala b/compiler/src/dotty/tools/dotc/transform/Memoize.scala index f46c3d14fbf9..b833d0cf1f6c 100644 --- a/compiler/src/dotty/tools/dotc/transform/Memoize.scala +++ b/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 { * --> 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))) diff --git a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala index 197a9f7a8ef8..d984b612ced2 100644 --- a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala +++ b/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, "") + + 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) = {