Skip to content

Commit

Permalink
Fix scala#9424: Add releaseFence() call when mixing in a trait val.
Browse files Browse the repository at this point in the history
This is a forward port of relevant parts of the upstream PR
scala/scala#7028
  • Loading branch information
sjrd committed Nov 26, 2020
1 parent e593e85 commit f256aa8
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 1 deletion.
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/core/StdNames.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
31 changes: 30 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/Memoize.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 = {
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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)))
Expand Down
87 changes: 87 additions & 0 deletions compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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) = {
Expand Down

0 comments on commit f256aa8

Please sign in to comment.