SI-7181 Prepare to remove duplicated finally blocks

As a first step towards fixing 7181, this commit improves the
comments and variable names around generating try/catch/finally blocks
in GenICode and adds a test verifying the current functionality of
try/catch/finally blocks

3 files changed, 157 insertions, 21 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
index 122972039b..8881650a81 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
@@ -1921,15 +1921,47 @@ abstract class GenICode extends SubComponent  {
        *   }), (AnotherExceptionClass,
        *   ctx => {...
        *   } ))`
+       *   
+       *   The resulting structure will look something like
+       *   
+       *   outer:
+       *     // this 'useless' jump will be removed later,
+       *     // for now it separates the try body's blocks from previous
+       *     // code since the try body needs its own exception handlers
+       *     JUMP body
+       *     
+       *   body:
+       *     [ try body ]
+       *     [ finally body ]
+       *     JUMP normalExit
+       *     
+       *   catch[i]:
+       *     [ handler[i] body ]
+       *     [ finally body ]
+       *     JUMP normalExit
+       *     
+       *   catchAll:
+       *     STORE exception
+       *     [ finally body ]
+       *     THROW exception
+       *     
+       *   normalExit:
+       *     
+       *  each catch[i] will cover body.  catchAll will cover both body and each catch[i]
+       *  Additional finally copies are created on the emission of every RETURN in the try body and exception handlers.
+       *  
+       *  This could result in unreachable code which has to be cleaned up later, e.g. if the try and all the exception
+       *  handlers always end in RETURN then there will be no "normal" flow out of the try/catch/finally.
+       *  Later reachability analysis will remove unreacahble code.
        */
       def Try(body: Context => Context,
               handlers: List[(Symbol, TypeKind, Context => Context)],
               finalizer: Tree,
               tree: Tree) = {
 
-        val outerCtx = this.dup       // context for generating exception handlers, covered by finalizer
+        val outerCtx = this.dup       // context for generating exception handlers, covered by the catch-all finalizer
         val finalizerCtx = this.dup   // context for generating finalizer handler
-        val afterCtx = outerCtx.newBlock()
+        val normalExitCtx = outerCtx.newBlock() // context where flow will go on a "normal" (non-return, non-throw) exit from a try or catch handler
         var tmp: Local = null
         val kind = toTypeKind(tree.tpe)
         val guardResult = kind != UNIT && mayCleanStack(finalizer)
@@ -1956,30 +1988,33 @@ abstract class GenICode extends SubComponent  {
         } else ctx
 
 
+        // Generate the catch-all exception handler that deals with uncaught exceptions coming
+        // from the try or exception handlers. It catches the exception, runs the finally code, then rethrows
+        // the exception
         if (finalizer != EmptyTree) {
           val exh = outerCtx.newExceptionHandler(NoSymbol, finalizer.pos) // finalizer covers exception handlers
           this.addActiveHandler(exh)  // .. and body aswell
-          val ctx = finalizerCtx.enterExceptionHandler(exh)
-          val exception = ctx.makeLocal(finalizer.pos, ThrowableClass.tpe, "exc")
-          loadException(ctx, exh, finalizer.pos)
-          ctx.bb.emit(STORE_LOCAL(exception))
-          val ctx1 = genLoad(finalizer, ctx, UNIT)
-          ctx1.bb.emit(LOAD_LOCAL(exception))
-          ctx1.bb.emit(THROW(ThrowableClass))
-          ctx1.bb.enterIgnoreMode()
-          ctx1.bb.close()
+          val exhStartCtx = finalizerCtx.enterExceptionHandler(exh)
+          val exception = exhStartCtx.makeLocal(finalizer.pos, ThrowableClass.tpe, "exc")
+          loadException(exhStartCtx, exh, finalizer.pos)
+          exhStartCtx.bb.emit(STORE_LOCAL(exception))
+          val exhEndCtx = genLoad(finalizer, exhStartCtx, UNIT)
+          exhEndCtx.bb.emit(LOAD_LOCAL(exception))
+          exhEndCtx.bb.closeWith(THROW(ThrowableClass))
+          exhEndCtx.bb.enterIgnoreMode()
           finalizerCtx.endHandler()
         }
 
+        // Generate each exception handler
         for ((sym, kind, handler) <- handlers) {
           val exh = this.newExceptionHandler(sym, tree.pos)
-          var ctx1 = outerCtx.enterExceptionHandler(exh)
-          ctx1.addFinalizer(finalizer, finalizerCtx)
-          loadException(ctx1, exh, tree.pos)
-          ctx1 = handler(ctx1)
+          val exhStartCtx = outerCtx.enterExceptionHandler(exh)
+          exhStartCtx.addFinalizer(finalizer, finalizerCtx)
+          loadException(exhStartCtx, exh, tree.pos)
+          val exhEndCtx = handler(exhStartCtx)
           // emit finalizer
-          val ctx2 = emitFinalizer(ctx1)
-          ctx2.bb.closeWith(JUMP(afterCtx.bb))
+          val exhEndCtx2 = emitFinalizer(exhEndCtx)
+          exhEndCtx2.bb.closeWith(JUMP(normalExitCtx.bb))
           outerCtx.endHandler()
         }
 
@@ -1987,14 +2022,14 @@ abstract class GenICode extends SubComponent  {
         if (finalizer != EmptyTree)
           bodyCtx.addFinalizer(finalizer, finalizerCtx)
 
-        var finalCtx = body(bodyCtx)
-        finalCtx = emitFinalizer(finalCtx)
+        var bodyEndCtx = body(bodyCtx)
+        bodyEndCtx = emitFinalizer(bodyEndCtx)
 
         outerCtx.bb.closeWith(JUMP(bodyCtx.bb))
 
-        finalCtx.bb.closeWith(JUMP(afterCtx.bb))
+        bodyEndCtx.bb.closeWith(JUMP(normalExitCtx.bb))
 
-        afterCtx
+        normalExitCtx
       }
     }
   }
diff --git a/test/files/run/t7181.check b/test/files/run/t7181.check
new file mode 100644
index 0000000000..e4b8e30dfe
--- /dev/null
+++ b/test/files/run/t7181.check
@@ -0,0 +1,23 @@
+normal exit MainNormalExit
+finally MainNormalExit
+normal flow MainNormalExit
+
+return MainReturn
+finally MainReturn
+
+uncaught exception MainUncaughtException
+finally MainUncaughtException
+
+caught exception ExceptionNormalExit
+normal exit ExceptionNormalExit
+finally ExceptionNormalExit
+normal flow ExceptionNormalExit
+
+caught exception ExceptionReturn
+return ExceptionReturn
+finally ExceptionReturn
+
+caught exception ExceptionUncaughtException
+uncaught exception ExceptionUncaughtException
+finally ExceptionUncaughtException
+
diff --git a/test/files/run/t7181.scala b/test/files/run/t7181.scala
new file mode 100644
index 0000000000..a055e43481
--- /dev/null
+++ b/test/files/run/t7181.scala
@@ -0,0 +1,78 @@
+sealed abstract class Action
+// exit the try body normally
+case object MainNormalExit extends Action
+// exit the try body with a 'return'
+case object MainReturn extends Action
+// exit the try body with an uncaught exception
+case object MainUncaughtException extends Action
+// exit the try body with a caught exception and exit the exception handler normally
+case object ExceptionNormalExit extends Action
+// exit the try body with a caught exception and exit the exception handler with a 'return'
+case object ExceptionReturn extends Action
+// exit the try body with a caught exception and exit the exception handler with an uncaught exception
+case object ExceptionUncaughtException extends Action
+
+case class UncaughtException(action: Action) extends RuntimeException
+case class CaughtException(action: Action) extends RuntimeException
+
+object Test extends App {
+  def test(action: Action, expectException: Boolean = false) {
+    var gotException = false
+    val result = try
+      driver(action)
+    catch {
+      case UncaughtException(a) => 
+        gotException = true
+        a
+    }
+    if (gotException) assert(expectException, "Got unexpected exception")
+    else assert(!expectException, "Did not get expected exception")
+
+    assert(result == action, s"Expected $action but got $result")
+    println()
+  }
+
+  def driver(action: Action): Action = {
+    val result = try {
+      action match {
+        case MainNormalExit => 
+          println(s"normal exit $action")
+          action
+        case MainReturn =>
+          println(s"return $action")
+          return action
+        case MainUncaughtException => 
+          println(s"uncaught exception $action")
+          throw UncaughtException(action)
+        case _ =>
+          println(s"caught exception $action")
+          throw CaughtException(action)
+      }
+    } catch {
+      case CaughtException(action) => action match {
+        case ExceptionNormalExit => 
+          println(s"normal exit $action")
+          action
+        case ExceptionReturn => 
+          println(s"return $action")
+          return action
+        case ExceptionUncaughtException => 
+          println(s"uncaught exception $action")
+          throw UncaughtException(action)
+        case _ => 
+          sys.error(s"unexpected $action in exception handler")
+      }
+    } finally {
+      println(s"finally $action")
+    }
+    println(s"normal flow $action")
+    result
+  }
+
+  test(MainNormalExit)
+  test(MainReturn)
+  test(MainUncaughtException, true)
+  test(ExceptionNormalExit)
+  test(ExceptionReturn)
+  test(ExceptionUncaughtException, true)
+}