Merge pull request #4872 from abeln/lift-try

Fix #4866: do not lift try with no catch block

Author: allanrenucci

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

@@ -20,6 +20,12 @@ import util.Store
  *  is lifted to
  *
  *      { def liftedTree$n() = try body catch handler; liftedTree$n() }
+ *
+ *  However, don't lift try's without catch expressions (try-finally).
+ *  Lifting is needed only for try-catch expressions that are evaluated in a context
+ *  where the stack might not be empty. `finally` does not attempt to continue evaluation
+ *  after an exception, so the fact that values on the stack are 'lost' does not matter
+ *  (copied from https://github.com/scala/scala/pull/922).
  */
 class LiftTry extends MiniPhase with IdentityDenotTransformer { thisPhase =>
   import ast.tpd._
@@ -58,7 +64,7 @@ class LiftTry extends MiniPhase with IdentityDenotTransformer { thisPhase =>
     liftingCtx(false)
 
   override def transformTry(tree: Try)(implicit ctx: Context): Tree =
-    if (needLift) {
+    if (needLift && tree.cases.nonEmpty) {
       ctx.debuglog(i"lifting tree at ${tree.pos}, current owner = ${ctx.owner}")
       val fn = ctx.newSymbol(
         ctx.owner, LiftedTreeName.fresh(), Synthetic | Method,

tests/run/i1692.scala

@@ -12,6 +12,9 @@ class LazyNullable(a: => Int) {
 
   private[this] val d = "D"
   lazy val l3 = d + d // null out d (Scalac require single use?)
+
+  private [this] val e = "E"
+  lazy val l4 = try e finally () // null out e
 }
 
 object LazyNullable2 {
@@ -47,6 +50,10 @@ class LazyNotNullable {
   class Inner {
     lazy val l8 = h
   }
+
+  private[this] val i = "I"
+  // not nullable because try is lifted, so i is used outside lazy val initializer
+  lazy val l9 = try i catch { case e: Exception => () } 
 }
 
 trait LazyTrait {
@@ -87,6 +94,9 @@ object Test {
     assert(lz.l3 == "DD")
     assertNull("d")
 
+    assert(lz.l4 == "E")
+    assertNull("e")
+
     assert(LazyNullable2.l0 == "A")
     assert(readField("a", LazyNullable2) == null)
   }
@@ -124,6 +134,9 @@ object Test {
     assert(inner.l8 == "H")
     assertNotNull("LazyNotNullable$$h") // fragile: test will break if compiler generated names change
 
+    assert(lz.l9 == "I")
+    assertNotNull("i")
+
     val fromTrait = new LazyTrait {}
     assert(fromTrait.l0 == "A")
     assert(readField("LazyTrait$$a", fromTrait) != null) // fragile: test will break if compiler generated names change

tests/run/i4866.check

@@ -0,0 +1,2 @@
+Foo #lifted: 0
+FooLifted #lifted: 1

tests/run/i4866.scala

@@ -0,0 +1,19 @@
+// Test that try-finally aren't lifted, but try-catch are.
+
+class Foo {
+  val field = try 1 finally ()
+}
+
+class FooLifted {
+  val field = try 1 catch { case e: Exception => () } finally () 
+}
+
+object Test extends App {
+  def numLifted(o: Object) = {
+    def isLifted(m: java.lang.reflect.Method) = m.getName.startsWith("lifted")
+    o.getClass.getDeclaredMethods.count(isLifted)
+  }
+  
+  println("Foo #lifted: " + numLifted(new Foo))
+  println("FooLifted #lifted: " + numLifted(new FooLifted))
+}