Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Nov 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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