Skip to content

Fix lift try and captured vars interaction #758

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

Merged
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
5 changes: 3 additions & 2 deletions src/dotty/tools/dotc/ast/tpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,8 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
def traverse(tree: Tree)(implicit ctx: Context) = tree match {
case tree: DefTree =>
val sym = tree.symbol
if (sym.denot(ctx.withPhase(trans)).owner == from) {
val prevDenot = sym.denot(ctx.withPhase(trans))
if (prevDenot.owner == from) {
val d = sym.copySymDenotation(owner = to)
d.installAfter(trans)
d.transformAfter(trans, d => if (d.owner eq from) d.copySymDenotation(owner = to) else d)
Expand All @@ -651,7 +652,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
traverseChildren(tree)
}
}
traverser.traverse(tree)
traverser.traverse(tree)(ctx.withMode(Mode.FutureDefsOK))
tree
}

Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/core/Decorators.scala
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ object Decorators {
*/
implicit class PhaseListDecorator(val names: List[String]) extends AnyVal {
def containsPhase(phase: Phase): Boolean = phase match {
case phase: TreeTransformer => phase.transformations.exists(trans => containsPhase(trans.phase))
case phase: TreeTransformer => phase.miniPhases.exists(containsPhase)
case _ =>
names exists { name =>
name == "all" || {
Expand Down
9 changes: 6 additions & 3 deletions src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -592,13 +592,16 @@ class Definitions {
}

/** The classes for which a Ref type exists. */
lazy val refClasses: collection.Set[Symbol] = ScalaNumericValueClasses + BooleanClass + ObjectClass
lazy val refClassKeys: collection.Set[Symbol] = ScalaNumericValueClasses + BooleanClass + ObjectClass

lazy val refClass: Map[Symbol, Symbol] =
refClasses.map(rc => rc -> ctx.requiredClass(s"scala.runtime.${rc.name}Ref")).toMap
refClassKeys.map(rc => rc -> ctx.requiredClass(s"scala.runtime.${rc.name}Ref")).toMap

lazy val volatileRefClass: Map[Symbol, Symbol] =
refClasses.map(rc => rc -> ctx.requiredClass(s"scala.runtime.Volatile${rc.name}Ref")).toMap
refClassKeys.map(rc => rc -> ctx.requiredClass(s"scala.runtime.Volatile${rc.name}Ref")).toMap

lazy val boxedRefClasses: collection.Set[Symbol] =
refClassKeys.flatMap(k => Set(refClass(k), volatileRefClass(k)))

def wrapArrayMethodName(elemtp: Type): TermName = {
val cls = elemtp.classSymbol
Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/core/Denotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ object Denotations {
// println(s"installing $this after $phase/${phase.id}, valid = ${current.validFor}")
// printPeriods(current)
this.validFor = Period(ctx.runId, targetId, current.validFor.lastPhaseId)
if (current.validFor.firstPhaseId == targetId)
if (current.validFor.firstPhaseId >= targetId)
replaceDenotation(current)
else {
// insert this denotation after current
Expand Down
18 changes: 8 additions & 10 deletions src/dotty/tools/dotc/core/Phases.scala
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,9 @@ object Phases {
assert(false, s"Only tree transforms can be squashed, ${phase.phaseName} can not be squashed")
}
}
val transforms = filteredPhaseBlock.asInstanceOf[List[MiniPhase]].map(_.treeTransform)
val block = new TreeTransformer {
override def phaseName: String = transformations.map(_.phase.phaseName).mkString("TreeTransform:{", ", ", "}")

override def transformations: Array[TreeTransform] = transforms.toArray
override def phaseName: String = miniPhases.map(_.phaseName).mkString("TreeTransform:{", ", ", "}")
override def miniPhases: Array[MiniPhase] = filteredPhaseBlock.asInstanceOf[List[MiniPhase]].toArray
}
prevPhases ++= filteredPhaseBlock.map(_.getClazz)
block
Expand Down Expand Up @@ -145,7 +143,7 @@ object Phases {
val flatPhases = collection.mutable.ListBuffer[Phase]()

phasess.foreach(p => p match {
case t: TreeTransformer => flatPhases ++= t.transformations.map(_.phase)
case t: TreeTransformer => flatPhases ++= t.miniPhases
case _ => flatPhases += p
})

Expand Down Expand Up @@ -173,11 +171,11 @@ object Phases {
val phase = phasess(i)
phase match {
case t: TreeTransformer =>
val transforms = t.transformations
transforms.foreach{ x =>
checkRequirements(x.phase)
x.phase.init(this, nextPhaseId)}
t.init(this, transforms.head.phase.id, transforms.last.phase.id)
val miniPhases = t.miniPhases
miniPhases.foreach{ phase =>
checkRequirements(phase)
phase.init(this, nextPhaseId)}
t.init(this, miniPhases.head.id, miniPhases.last.id)
case _ =>
phase.init(this, nextPhaseId)
checkRequirements(phase)
Expand Down
32 changes: 26 additions & 6 deletions src/dotty/tools/dotc/transform/CapturedVars.scala
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,34 @@ class CapturedVars extends MiniPhase with IdentityDenotTransformer { thisTransfo
else id
}

/** If assignment is to a boxed ref type, e.g.
*
* intRef.elem = expr
*
* rewrite using a temporary var to
*
* val ev$n = expr
* intRef.elem = ev$n
*
* That way, we avoid the problem that `expr` might contain a `try` that would
* run on a non-empty stack (which is illegal under JVM rules). Note that LiftTry
* has already run before, so such `try`s would not be eliminated.
*
* Also: If the ref type lhs is followed by a cast (can be an artifact of nested translation),
* drop the cast.
*/
override def transformAssign(tree: Assign)(implicit ctx: Context, info: TransformerInfo): Tree = {
val lhs1 = tree.lhs match {
case TypeApply(Select(qual @ Select(qual2, nme.elem), nme.asInstanceOf_), _) =>
assert(captured(qual2.symbol))
qual
case _ => tree.lhs
def recur(lhs: Tree): Tree = lhs match {
case TypeApply(Select(qual, nme.asInstanceOf_), _) =>
val Select(_, nme.elem) = qual
recur(qual)
case Select(_, nme.elem) if defn.boxedRefClasses.contains(lhs.symbol.maybeOwner) =>
val tempDef = transformFollowing(SyntheticValDef(ctx.freshName("ev$").toTermName, tree.rhs))
transformFollowing(Block(tempDef :: Nil, cpy.Assign(tree)(lhs, ref(tempDef.symbol))))
case _ =>
tree
}
cpy.Assign(tree)(lhs1, tree.rhs)
recur(tree.lhs)
}
}
}
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/transform/TreeChecker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class TreeChecker extends Phase with SymTransformer {

private def previousPhases(phases: List[Phase])(implicit ctx: Context): List[Phase] = phases match {
case (phase: TreeTransformer) :: phases1 =>
val subPhases = phase.transformations.map(_.phase)
val subPhases = phase.miniPhases
val previousSubPhases = previousPhases(subPhases.toList)
if (previousSubPhases.length == subPhases.length) previousSubPhases ::: previousPhases(phases1)
else previousSubPhases
Expand Down
23 changes: 11 additions & 12 deletions src/dotty/tools/dotc/transform/TreeTransform.scala
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ object TreeTransforms {

def treeTransformPhase: Phase = phase.next

/** id of this treeTransform in group */
var idx: Int = _

def prepareForIdent(tree: Ident)(implicit ctx: Context) = this
def prepareForSelect(tree: Select)(implicit ctx: Context) = this
def prepareForThis(tree: This)(implicit ctx: Context) = this
Expand Down Expand Up @@ -137,10 +134,10 @@ object TreeTransforms {
def transform(tree: Tree)(implicit ctx: Context, info: TransformerInfo): Tree = info.group.transform(tree, info, 0)

/** Transform subtree using all transforms following the current one in this group */
def transformFollowingDeep(tree: Tree)(implicit ctx: Context, info: TransformerInfo): Tree = info.group.transform(tree, info, idx + 1)
def transformFollowingDeep(tree: Tree)(implicit ctx: Context, info: TransformerInfo): Tree = info.group.transform(tree, info, phase.idx + 1)

/** Transform single node using all transforms following the current one in this group */
def transformFollowing(tree: Tree)(implicit ctx: Context, info: TransformerInfo): Tree = info.group.transformSingle(tree, idx + 1)
def transformFollowing(tree: Tree)(implicit ctx: Context, info: TransformerInfo): Tree = info.group.transformSingle(tree, phase.idx + 1)

def atGroupEnd[T](action : Context => T)(implicit ctx: Context, info: TransformerInfo) = {
val last = info.transformers(info.transformers.length - 1)
Expand All @@ -152,14 +149,17 @@ object TreeTransforms {
trait MiniPhase extends Phase { thisPhase =>
def treeTransform: TreeTransform

/** id of this mini phase in group */
var idx: Int = _

/** List of names of phases that should have finished their processing of all compilation units
* before this phase starts
*/
def runsAfterGroupsOf: Set[Class[_ <: Phase]] = Set.empty

protected def mkTreeTransformer = new TreeTransformer {
override def phaseName: String = thisPhase.phaseName
override def transformations = Array(treeTransform)
override def miniPhases = Array(thisPhase)
}

override def run(implicit ctx: Context): Unit = {
Expand Down Expand Up @@ -197,7 +197,6 @@ object TreeTransforms {

@sharable val NoTransform = new TreeTransform {
def phase = unsupported("phase")
idx = -1
}

type Mutator[T] = (TreeTransform, T, Context) => TreeTransform
Expand Down Expand Up @@ -474,7 +473,7 @@ object TreeTransforms {
/** A group of tree transforms that are applied in sequence during the same phase */
abstract class TreeTransformer extends Phase {

def transformations: Array[TreeTransform]
def miniPhases: Array[MiniPhase]

override def run(implicit ctx: Context): Unit = {
val curTree = ctx.compilationUnit.tpdTree
Expand Down Expand Up @@ -549,10 +548,10 @@ object TreeTransforms {
val prepForStats: Mutator[List[Tree]] = (trans, trees, ctx) => trans.prepareForStats(trees)(ctx)
val prepForUnit: Mutator[Tree] = (trans, tree, ctx) => trans.prepareForUnit(tree)(ctx)

val initialTransformationsCache = transformations.zipWithIndex.map {
case (transform, id) =>
transform.idx = id
transform
val initialTransformationsCache = miniPhases.zipWithIndex.map {
case (miniPhase, id) =>
miniPhase.idx = id
miniPhase.treeTransform
}

val initialInfoCache = new TransformerInfo(initialTransformationsCache, new NXTransformations(initialTransformationsCache), this)
Expand Down
10 changes: 5 additions & 5 deletions test/test/transform/TreeTransformerTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class TreeTransformerTest extends DottyTest {
init(ctx, ctx.period.firstPhaseId, ctx.period.lastPhaseId)
}
val transformer = new TreeTransformer {
override def transformations = Array(new EmptyTransform)
override def miniPhases = Array(new EmptyTransform)

override def phaseName: String = "test"
}
Expand All @@ -42,7 +42,7 @@ class TreeTransformerTest extends DottyTest {
init(ctx, ctx.period.firstPhaseId, ctx.period.lastPhaseId)
}
val transformer = new TreeTransformer {
override def transformations = Array(new ConstantTransform)
override def miniPhases = Array(new ConstantTransform)

override def phaseName: String = "test"
}
Expand Down Expand Up @@ -72,7 +72,7 @@ class TreeTransformerTest extends DottyTest {
init(ctx, ctx.period.firstPhaseId, ctx.period.lastPhaseId)
}
val transformer = new TreeTransformer {
override def transformations = Array(new Transformation)
override def miniPhases = Array(new Transformation)

override def phaseName: String = "test"

Expand Down Expand Up @@ -119,7 +119,7 @@ class TreeTransformerTest extends DottyTest {
init(ctx, ctx.period.firstPhaseId, ctx.period.lastPhaseId)
}
val transformer = new TreeTransformer {
override def transformations = Array(new Transformation1, new Transformation2)
override def miniPhases = Array(new Transformation1, new Transformation2)

override def phaseName: String = "test"
}
Expand Down Expand Up @@ -187,7 +187,7 @@ class TreeTransformerTest extends DottyTest {
init(ctx, ctx.period.firstPhaseId, ctx.period.lastPhaseId)
}
val transformer = new TreeTransformer {
override def transformations = Array(new Transformation1, new Transformation2)
override def miniPhases = Array(new Transformation1, new Transformation2)

override def phaseName: String = "test"
}
Expand Down
12 changes: 11 additions & 1 deletion tests/run/liftedTry.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,20 @@ object Test {

foo(try 3 catch handle)

def main(args: Array[String]) = {
def main(args: Array[String]): Unit = {
assert(x == 1)
assert(foo(2) == 2)
assert(foo(try raise(3) catch handle) == 3)
Tr.foo
}
}


object Tr {
def fun(a: Int => Unit) = a(2)
def foo: Int = {
var s = 1
s = try {fun(s = _); 3} catch{ case ex: Throwable => val x = 4; s = x; 5 }
s
}
}