Merge pull request #758 from dotty-staging/fix-liftedTry-capturedVars-interaction

Fix lift try and captured vars interaction

Author: DarkDimius

src/dotty/tools/dotc/ast/tpd.scala

@@ -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)
@@ -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
     }
 

src/dotty/tools/dotc/core/Decorators.scala

@@ -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" || {

src/dotty/tools/dotc/core/Definitions.scala

@@ -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

src/dotty/tools/dotc/core/Denotations.scala

@@ -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

src/dotty/tools/dotc/core/Phases.scala

@@ -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
@@ -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
       })
 
@@ -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)

src/dotty/tools/dotc/transform/CapturedVars.scala

@@ -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)
     }
   }
 }

src/dotty/tools/dotc/transform/TreeChecker.scala

@@ -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

src/dotty/tools/dotc/transform/TreeTransform.scala

@@ -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
@@ -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)
@@ -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 = {
@@ -197,7 +197,6 @@ object TreeTransforms {
 
   @sharable val NoTransform = new TreeTransform {
     def phase = unsupported("phase")
-    idx = -1
   }
 
   type Mutator[T] = (TreeTransform, T, Context) => TreeTransform
@@ -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
@@ -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)

test/test/transform/TreeTransformerTest.scala

@@ -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"
       }
@@ -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"
       }
@@ -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"
 
@@ -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"
       }
@@ -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"
       }

tests/run/liftedTry.scala

@@ -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
+  }
+}