From a0a1c8f04091d747752f57264ef88695fb0a3818 Mon Sep 17 00:00:00 2001
From: Abel Nieto <abel.nieto90@gmail.com>
Date: Mon, 30 Jul 2018 18:37:05 +0200
Subject: [PATCH] Fix #4866: do not lift try with no catch block

Forward port of https://github.com/scala/scala/pull/922
---
 .../dotty/tools/dotc/transform/LiftTry.scala  |  8 +++++++-
 tests/run/i1692.scala                         | 13 +++++++++++++
 tests/run/i4866.check                         |  2 ++
 tests/run/i4866.scala                         | 19 +++++++++++++++++++
 4 files changed, 41 insertions(+), 1 deletion(-)
 create mode 100644 tests/run/i4866.check
 create mode 100644 tests/run/i4866.scala

diff --git a/compiler/src/dotty/tools/dotc/transform/LiftTry.scala b/compiler/src/dotty/tools/dotc/transform/LiftTry.scala
index f42131b88153..c801779232d6 100644
--- a/compiler/src/dotty/tools/dotc/transform/LiftTry.scala
+++ b/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,
diff --git a/tests/run/i1692.scala b/tests/run/i1692.scala
index f70cd1b2ed16..ce51c215d7fb 100644
--- a/tests/run/i1692.scala
+++ b/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
diff --git a/tests/run/i4866.check b/tests/run/i4866.check
new file mode 100644
index 000000000000..f16e2a9c94df
--- /dev/null
+++ b/tests/run/i4866.check
@@ -0,0 +1,2 @@
+Foo #lifted: 0
+FooLifted #lifted: 1
diff --git a/tests/run/i4866.scala b/tests/run/i4866.scala
new file mode 100644
index 000000000000..8e131e76df91
--- /dev/null
+++ b/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))
+}