Fixed several issues with finally, closes #3965.

3 files changed, 145 insertions, 19 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
index ad67eb3025..64170482ce 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
@@ -673,21 +673,22 @@ abstract class GenICode extends SubComponent  {
           var ctx1         = genLoad(expr, ctx, returnedKind)
           lazy val tmp     = ctx1.makeLocal(tree.pos, expr.tpe, "tmp")
           val saved        = savingCleanups(ctx1) {
-            ctx1.cleanups exists {
+            var saved = false
+            ctx1.cleanups foreach {
               case MonitorRelease(m) =>
                 if (settings.debug.value)
                   log("removing " + m + " from cleanups: " + ctx1.cleanups)
                 ctx1.bb.emit(Seq(LOAD_LOCAL(m), MONITOR_EXIT()))
                 ctx1.exitSynchronized(m)
-                false
-              case Finalizer(f) =>
+
+              case Finalizer(f, finalizerCtx) =>
                 if (settings.debug.value)
                   log("removing " + f + " from cleanups: " + ctx1.cleanups)
 
-                val saved = returnedKind != UNIT && mayCleanStack(f) && {
+                if (returnedKind != UNIT && mayCleanStack(f)) {
                   log("Emitting STORE_LOCAL for " + tmp + " to save finalizer.")
                   ctx1.bb.emit(STORE_LOCAL(tmp))
-                  true
+                  saved = true
                 }
 
                 // duplicate finalizer (takes care of anchored labels)
@@ -696,9 +697,11 @@ abstract class GenICode extends SubComponent  {
                 // we have to run this without the same finalizer in
                 // the list, otherwise infinite recursion happens for
                 // finalizers that contain 'return'
-                ctx1 = genLoad(f1, ctx1.removeFinalizer(f), UNIT)
-                saved
+                val fctx = finalizerCtx.newBlock
+                ctx1.bb.closeWith(JUMP(fctx.bb))
+                ctx1 = genLoad(f1, fctx, UNIT)
             }
+            saved
           }
 
           if (saved) {
@@ -1838,7 +1841,7 @@ abstract class GenICode extends SubComponent  {
       def contains(x: AnyRef) = value == x
     }
     case class MonitorRelease(m: Local) extends Cleanup(m) { }
-    case class Finalizer(f: Tree) extends Cleanup (f) { }
+    case class Finalizer(f: Tree, ctx: Context) extends Cleanup (f) { }
 
     def duplicateFinalizer(boundLabels: Set[Symbol], targetCtx: Context, finalizer: Tree) =  {
       (new DuplicateLabels(boundLabels))(targetCtx, finalizer)
@@ -1952,8 +1955,8 @@ abstract class GenICode extends SubComponent  {
         this
       }
 
-      def addFinalizer(f: Tree): this.type = {
-        cleanups = Finalizer(f) :: cleanups;
+      def addFinalizer(f: Tree, ctx: Context): this.type = {
+        cleanups = Finalizer(f, ctx) :: cleanups;
         this
       }
 
@@ -2134,7 +2137,7 @@ abstract class GenICode extends SubComponent  {
         val exhs = handlers.map { handler =>
             val exh = this.newExceptionHandler(handler._1, handler._2, tree.pos)
             var ctx1 = outerCtx.enterExceptionHandler(exh)
-            ctx1.addFinalizer(finalizer)
+            ctx1.addFinalizer(finalizer, finalizerCtx)
             loadException(ctx1, exh, tree.pos)
             ctx1 = handler._3(ctx1)
             // emit finalizer
@@ -2145,7 +2148,7 @@ abstract class GenICode extends SubComponent  {
           }
         val bodyCtx = this.newBlock
         if (finalizer != EmptyTree)
-          bodyCtx.addFinalizer(finalizer)
+          bodyCtx.addFinalizer(finalizer, finalizerCtx)
 
         var finalCtx = body(bodyCtx)
         finalCtx = emitFinalizer(finalCtx)
diff --git a/test/files/run/finally.check b/test/files/run/finally.check
index fa565c8e8a..901a797426 100644
--- a/test/files/run/finally.check
+++ b/test/files/run/finally.check
@@ -1,5 +1,35 @@
+Running throwCatchFinally
 hi
 In Finally
 java.lang.RuntimeException: ouch
+----------------------------------------
+Running retCatch
 java.lang.Exception
 in finally
+----------------------------------------
+Running throwCatch
+java.lang.Exception
+in finally
+COUGHT: java.lang.Exception
+----------------------------------------
+Running retBody
+in finally
+----------------------------------------
+Running throwBody
+java.lang.Exception
+in finally
+----------------------------------------
+Running retFinally
+body
+in finally 1
+in finally 2
+----------------------------------------
+Running throwFinally
+body
+in finally
+java.lang.Exception
+----------------------------------------
+Running nestedFinalies
+in finally 1
+in finally 2
+----------------------------------------
diff --git a/test/files/run/finally.scala b/test/files/run/finally.scala
index 0da616cfdd..b3b7f684e5 100644
--- a/test/files/run/finally.scala
+++ b/test/files/run/finally.scala
@@ -1,6 +1,17 @@
 
-// test that finally is not covered by any exception handlers.
 object Test extends App {
+
+
+  // test that finally is not covered by any exception handlers.
+  def throwCatchFinally {
+    try {
+      bar
+    } catch {
+      case e => println(e)
+    }
+  }
+
+  // test that finally is not covered by any exception handlers.
   def bar {
     try {
       println("hi")
@@ -14,7 +25,8 @@ object Test extends App {
     }
   }
 
-  def m1 {
+  // return in catch (finally is executed)
+  def retCatch {
     try {
       throw new Exception
     } catch {
@@ -24,11 +36,92 @@ object Test extends App {
     } finally println("in finally")
   }
 
-  try {
-    bar
-  } catch {
-    case e => println(e)
+  // throw in catch (finally is executed, exception propagated)
+  def throwCatch {
+    try {
+      throw new Exception
+    } catch {
+      case e =>
+        println(e);
+        throw e
+    } finally println("in finally")
+  }
+
+  // return inside body (finally is executed)
+  def retBody {
+    try {
+      return
+    } catch {
+      case e =>
+        println(e);
+        throw e
+    } finally println("in finally")
+  }
+
+  // throw inside body (finally and catch are executed)
+  def throwBody {
+    try {
+      throw new Exception
+    } catch {
+      case e =>
+        println(e);
+    } finally println("in finally")
+  }
+
+  // return inside finally (each finally is executed once)
+  def retFinally {
+    try {
+      try println("body")
+      finally {
+        println("in finally 1")
+        return
+      }
+    } finally println("in finally 2")
+  }
+
+
+  // throw inside finally (finally is executed once, exception is propagated)
+  def throwFinally {
+    try {
+      try println("body")
+      finally {
+        println("in finally")
+        throw new Exception
+      }
+    } catch {
+      case e => println(e)
+    }
+  }
+
+  // nested finallies with return value
+  def nestedFinalies: Int =
+    try {
+      try {
+        return 10
+      } finally {
+        try { () } catch { case _ => () }
+        println("in finally 1")
+      }
+    } finally {
+      println("in finally 2")
+    }
+
+  def test[A](m: => A, name: String) {
+    println("Running %s".format(name))
+    try {
+      m
+    } catch {
+      case e => println("COUGHT: " + e)
+    }
+    println("-" * 40)
   }
 
-  m1
+  test(throwCatchFinally, "throwCatchFinally")
+  test(retCatch, "retCatch")
+  test(throwCatch, "throwCatch")
+  test(retBody, "retBody")
+  test(throwBody, "throwBody")
+  test(retFinally, "retFinally")
+  test(throwFinally, "throwFinally")
+  test(nestedFinalies, "nestedFinalies")
 }