Merge pull request #7846 from dotty-staging/fix-#7843

Fix #7843: Move LiftTry back to before CapturedVars

Author: odersky

compiler/src/dotty/tools/dotc/Compiler.scala

@@ -83,6 +83,7 @@ class Compiler {
          new InterceptedMethods,     // Special handling of `==`, `|=`, `getClass` methods
          new Getters,                // Replace non-private vals and vars with getter defs (fields are added later)
          new ElimByName,             // Expand by-name parameter references
+         new LiftTry,                // Put try expressions that might execute on non-empty stacks into their own methods
          new CollectNullableFields,  // Collect fields that can be nulled out after use in lazy initialization
          new ElimOuterSelect,        // Expand outer selections
          new AugmentScala2Traits,    // Augments Scala2 traits with additional members needed for mixin composition.
@@ -105,8 +106,7 @@ class Compiler {
                                         // Note: constructors changes decls in transformTemplate, no InfoTransformers should be added after it
          new FunctionalInterfaces,   // Rewrites closures to implement @specialized types of Functions.
          new Instrumentation,        // Count closure allocations under -Yinstrument-closures
-         new GetClass,               // Rewrites getClass calls on primitive types.
-         new LiftTry) ::             // Put try expressions that might execute on non-empty stacks into their own methods their implementations
+         new GetClass) ::            // Rewrites getClass calls on primitive types.
     List(new LinkScala2Impls,        // Redirect calls to trait methods defined by Scala 2.x, so that they now go to
          new LambdaLift,             // Lifts out nested functions to class scope, storing free variables in environments
                                         // Note: in this mini-phase block scopes are incorrect. No phases that rely on scopes should be here

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

@@ -23,6 +23,9 @@ class CapturedVars extends MiniPhase with IdentityDenotTransformer { thisPhase =
   /** the following two members override abstract members in Transform */
   val phaseName: String = "capturedVars"
 
+  override def runsAfterGroupsOf: Set[String] = Set(LiftTry.name)
+    // lifting tries changes what variables are considered to be captured
+
   private[this] var Captured: Store.Location[collection.Set[Symbol]] = _
   private def captured(implicit ctx: Context) = ctx.store(Captured)
 

compiler/src/dotty/tools/dotc/transform/LiftTry.scala

@@ -30,10 +30,7 @@ import util.Store
 class LiftTry extends MiniPhase with IdentityDenotTransformer { thisPhase =>
   import ast.tpd._
 
-  val phaseName: String = "liftTry"
-
-  // See tests/run/t2333.scala for an example where running after the group of LazyVals matters
-  override def runsAfterGroupsOf: Set[String] = Set(LazyVals.name)
+  val phaseName: String = LiftTry.name
 
   private var NeedLift: Store.Location[Boolean] = _
   private def needLift(implicit ctx: Context): Boolean = ctx.store(NeedLift)
@@ -48,9 +45,15 @@ class LiftTry extends MiniPhase with IdentityDenotTransformer { thisPhase =>
     liftingCtx(true)
 
   override def prepareForValDef(tree: ValDef)(implicit ctx: Context): Context =
-    if (!tree.symbol.exists  ||
-        tree.symbol.isSelfSym ||
-        tree.symbol.owner == ctx.owner.enclosingMethod) ctx
+    if !tree.symbol.exists
+       || tree.symbol.isSelfSym
+       || tree.symbol.owner == ctx.owner.enclosingMethod
+          && !tree.symbol.is(Lazy)
+            // The current implementation wraps initializers of lazy vals in
+            // calls to an initialize method, which means that a `try` in the
+            // initializer needs to be lifted. Note that the new scheme proposed
+            // in #6979 would avoid this.
+    then ctx
     else liftingCtx(true)
 
   override def prepareForAssign(tree: Assign)(implicit ctx: Context): Context =
@@ -75,3 +78,5 @@ class LiftTry extends MiniPhase with IdentityDenotTransformer { thisPhase =>
     }
     else tree
 }
+object LiftTry with
+  val name = "liftTry"

tests/run/i7843.check

@@ -0,0 +1,4 @@
+done outside try: false
+setting done to true
+done in catch clause: true
+done

tests/run/i7843.scala

@@ -0,0 +1,34 @@
+import scala.concurrent._, duration._
+
+object Test {
+  implicit val ec: scala.concurrent.ExecutionContext = scala.concurrent.ExecutionContext.global
+  val thunkFuture: Future[() => String] = Future.successful(() => throw new RuntimeException("Whoops"))
+
+  final def shouldTerminate: Future[Unit] =
+    thunkFuture.flatMap { thunk =>
+      def fold() = {
+        var done = false
+        while (!done) {
+          println(s"done outside try: $done")
+          try {
+            thunk()
+          } catch {
+            case _: Throwable =>
+              println("setting done to true")
+              done = true
+              println(s"done in catch clause: $done")
+          }
+        }
+      }
+
+      Future {
+        fold()
+      }
+    }
+
+
+  def main(args: Array[String]): Unit = {
+    Await.result(shouldTerminate, Duration.Inf)
+    println("done")
+  }
+}