Merge pull request #2169 from JamesIry/master_SI-7181

SI-7181 Eliminate unnecessary duplicates of finally blocks

5 files changed, 226 insertions, 75 deletions

diff --git a/test/files/jvm/t7181/Foo_1.scala b/test/files/jvm/t7181/Foo_1.scala
new file mode 100644
index 0000000000..f9dfdd4442
--- /dev/null
+++ b/test/files/jvm/t7181/Foo_1.scala
@@ -0,0 +1,26 @@
+class Exception1 extends RuntimeException
+class Exception2 extends RuntimeException
+
+class Foo_1 {
+  def foo(baz: Baz) {
+    try {
+      baz.bar
+    } catch {
+      case _: Exception1 => println("exception 1")
+      case _: Exception2 => println("exception 2")
+    } finally {
+      // this should be the only copy of the magic constant 3
+      // making it easy to detect copies of this finally block
+      println(s"finally ${3}")
+    }
+    println(s"normal flow")
+  }
+}
+
+trait Baz {
+  // does it throw? who knows? This way
+  // I can ensure that no optimization that honors
+  // separate compilation could ever
+  // change the exception handling structure
+  def bar: Unit
+}
diff --git a/test/files/jvm/t7181/Test.scala b/test/files/jvm/t7181/Test.scala
new file mode 100644
index 0000000000..35dba436c1
--- /dev/null
+++ b/test/files/jvm/t7181/Test.scala
@@ -0,0 +1,24 @@
+import scala.tools.partest.BytecodeTest
+import scala.tools.asm
+import asm.tree.InsnList
+import scala.collection.JavaConverters._
+
+object Test extends BytecodeTest {
+  def show: Unit = {
+    val classNode = loadClassNode("Foo_1")
+    val methodNode = getMethod(classNode, "foo")
+    // there should be 2 copies of the finally block, each with the magic constant 3
+    // one for the "normal" exit
+    // one for the uncaught exception exit
+    // prior to this PR there would have been 4 since each exception handler would also get a copy
+    val expected = 2
+    val got = countMagicThrees(methodNode.instructions)
+    assert(got == expected, s"expected $expected but got $got magic threes")
+  }
+
+  def countMagicThrees(insnList: InsnList): Int = {
+    def isMagicThree(node: asm.tree.AbstractInsnNode): Boolean =
+      (node.getOpcode == asm.Opcodes.ICONST_3)
+    insnList.iterator.asScala.count(isMagicThree)
+  }
+}
diff --git a/test/files/run/inline-ex-handlers.check b/test/files/run/inline-ex-handlers.check
index f2f0b60687..0a234e2659 100644
--- a/test/files/run/inline-ex-handlers.check
+++ b/test/files/run/inline-ex-handlers.check
@@ -107,27 +107,27 @@
 ---
 >     catch (Throwable) in ArrayBuffer(4, 6, 7, 8, 9, 10, 11, 12) starting at: 3
 619c642
-<   blocks: [1,2,3,4,5,6,7,9,10]
+<   blocks: [1,3,4,5,6,8,9]
 ---
->   blocks: [1,2,3,4,5,6,7,9,10,11,12]
+>   blocks: [1,3,4,5,6,8,9,10,11]
 643c666,667
 <     78	THROW(IllegalArgumentException)
 ---
 >     ?	STORE_LOCAL(value e)
->     ?	JUMP 11
+>     ?	JUMP 10
 644a669,673
->   11: 
+>   10: 
 >     81	LOAD_LOCAL(value e)
 >     ?	STORE_LOCAL(variable exc1)
->     ?	JUMP 12
+>     ?	JUMP 11
 >     
-672c701,702
+669c698,699
 <     81	THROW(Exception)
 ---
 >     ?	STORE_LOCAL(variable exc1)
->     ?	JUMP 12
-688a719,731
->   12: 
+>     ?	JUMP 11
+685a716,728
+>   11: 
 >     83	LOAD_MODULE object Predef
 >     83	CONSTANT("finally")
 >     83	CALL_METHOD scala.Predef.println (dynamic)
@@ -140,88 +140,88 @@
 >     84	LOAD_LOCAL(variable exc1)
 >     84	THROW(Throwable)
 >     
-694c737
-<     catch (<none>) in ArrayBuffer(4, 6, 7, 9) starting at: 3
+691c734
+<     catch (<none>) in ArrayBuffer(4, 5, 6, 8) starting at: 3
 ---
->     catch (<none>) in ArrayBuffer(4, 6, 7, 9, 11) starting at: 3
-718c761
+>     catch (<none>) in ArrayBuffer(4, 5, 6, 8, 10) starting at: 3
+715c758
 <   locals: value args, variable result, value ex6, variable exc2, value x4, value x5, value message, value x, value ex6, value x4, value x5, value message, value x
 ---
 >   locals: value args, variable result, value ex6, variable exc2, value x4, value x5, value x, value ex6, value x4, value x5, value x
-720c763
-<   blocks: [1,2,3,4,5,6,9,11,14,15,16,19,21,22,24,25]
+717c760
+<   blocks: [1,3,4,5,6,9,13,14,15,18,20,21,23,24]
 ---
->   blocks: [1,2,3,4,5,6,9,11,14,15,16,19,21,22,24,25,26,27,28]
-744c787,794
+>   blocks: [1,3,4,5,6,9,13,14,15,18,20,21,23,24,25,26,27]
+741c784,791
 <     172	THROW(MyException)
 ---
 >     ?	STORE_LOCAL(value ex6)
->     ?	JUMP 26
+>     ?	JUMP 25
 >     
->   26: 
+>   25: 
 >     170	LOAD_LOCAL(value ex6)
 >     170	STORE_LOCAL(value x4)
 >     170	SCOPE_ENTER value x4
->     170	JUMP 15
-787,790d836
+>     170	JUMP 14
+781,784d830
 <     175	LOAD_LOCAL(value x5)
 <     175	CALL_METHOD MyException.message (dynamic)
 <     175	STORE_LOCAL(value message)
 <     175	SCOPE_ENTER value message
-792c838,839
+786c832,833
 <     176	LOAD_LOCAL(value message)
 ---
 >     ?	LOAD_LOCAL(value x5)
 >     176	CALL_METHOD MyException.message (dynamic)
-796c843,844
+790c837,838
 <     177	LOAD_LOCAL(value message)
 ---
 >     ?	LOAD_LOCAL(value x5)
 >     177	CALL_METHOD MyException.message (dynamic)
-798c846,847
+792c840,841
 <     177	THROW(MyException)
 ---
 >     ?	STORE_LOCAL(value ex6)
->     ?	JUMP 27
-802c851,852
+>     ?	JUMP 26
+796c845,846
 <     170	THROW(Throwable)
 ---
 >     ?	STORE_LOCAL(value ex6)
->     ?	JUMP 27
-811a862,867
->   27: 
+>     ?	JUMP 26
+805a856,861
+>   26: 
 >     169	LOAD_LOCAL(value ex6)
 >     169	STORE_LOCAL(value x4)
 >     169	SCOPE_ENTER value x4
 >     169	JUMP 5
 >     
-822,825d877
+816,819d871
 <     180	LOAD_LOCAL(value x5)
 <     180	CALL_METHOD MyException.message (dynamic)
 <     180	STORE_LOCAL(value message)
 <     180	SCOPE_ENTER value message
-827c879,880
+821c873,874
 <     181	LOAD_LOCAL(value message)
 ---
 >     ?	LOAD_LOCAL(value x5)
 >     181	CALL_METHOD MyException.message (dynamic)
-831c884,885
+825c878,879
 <     182	LOAD_LOCAL(value message)
 ---
 >     ?	LOAD_LOCAL(value x5)
 >     182	CALL_METHOD MyException.message (dynamic)
-833c887,888
+827c881,882
 <     182	THROW(MyException)
 ---
 >     ?	STORE_LOCAL(variable exc2)
->     ?	JUMP 28
-837c892,893
+>     ?	JUMP 27
+831c886,887
 <     169	THROW(Throwable)
 ---
 >     ?	STORE_LOCAL(variable exc2)
->     ?	JUMP 28
-853a910,922
->   28: 
+>     ?	JUMP 27
+847a904,916
+>   27: 
 >     184	LOAD_MODULE object Predef
 >     184	CONSTANT("finally")
 >     184	CALL_METHOD scala.Predef.println (dynamic)
@@ -234,23 +234,23 @@
 >     185	LOAD_LOCAL(variable exc2)
 >     185	THROW(Throwable)
 >     
-859c928
-<     catch (Throwable) in ArrayBuffer(14, 15, 16, 19, 21, 22, 24) starting at: 4
+853c922
+<     catch (Throwable) in ArrayBuffer(13, 14, 15, 18, 20, 21, 23) starting at: 4
 ---
->     catch (Throwable) in ArrayBuffer(14, 15, 16, 19, 21, 22, 24, 26) starting at: 4
-862c931
-<     catch (<none>) in ArrayBuffer(4, 5, 6, 9, 14, 15, 16, 19, 21, 22, 24) starting at: 3
+>     catch (Throwable) in ArrayBuffer(13, 14, 15, 18, 20, 21, 23, 25) starting at: 4
+856c925
+<     catch (<none>) in ArrayBuffer(4, 5, 6, 9, 13, 14, 15, 18, 20, 21, 23) starting at: 3
 ---
->     catch (<none>) in ArrayBuffer(4, 5, 6, 9, 14, 15, 16, 19, 21, 22, 24, 26, 27) starting at: 3
-886c955
+>     catch (<none>) in ArrayBuffer(4, 5, 6, 9, 13, 14, 15, 18, 20, 21, 23, 25, 26) starting at: 3
+880c949
 <   locals: value args, variable result, value e, value ex6, value x4, value x5, value message, value x
 ---
 >   locals: value args, variable result, value e, value ex6, value x4, value x5, value x
-888c957
+882c951
 <   blocks: [1,2,3,6,7,8,11,13,14,16]
 ---
 >   blocks: [1,2,3,6,7,8,11,13,14,16,17]
-912c981,988
+906c975,982
 <     124	THROW(MyException)
 ---
 >     ?	STORE_LOCAL(value ex6)
@@ -261,29 +261,29 @@
 >     122	STORE_LOCAL(value x4)
 >     122	SCOPE_ENTER value x4
 >     122	JUMP 7
-937,940d1012
+931,934d1006
 <     127	LOAD_LOCAL(value x5)
 <     127	CALL_METHOD MyException.message (dynamic)
 <     127	STORE_LOCAL(value message)
 <     127	SCOPE_ENTER value message
-942c1014,1015
+936c1008,1009
 <     127	LOAD_LOCAL(value message)
 ---
 >     ?	LOAD_LOCAL(value x5)
 >     127	CALL_METHOD MyException.message (dynamic)
-971c1044
+965c1038
 <     catch (IllegalArgumentException) in ArrayBuffer(6, 7, 8, 11, 13, 14, 16) starting at: 3
 ---
 >     catch (IllegalArgumentException) in ArrayBuffer(6, 7, 8, 11, 13, 14, 16, 17) starting at: 3
-995c1068
+989c1062
 <   locals: value args, variable result, value ex6, value x4, value x5, value message, value x, value e
 ---
 >   locals: value args, variable result, value ex6, value x4, value x5, value x, value e
-997c1070
+991c1064
 <   blocks: [1,2,3,4,5,8,12,13,14,16]
 ---
 >   blocks: [1,2,3,5,8,12,13,14,16,17]
-1021c1094,1103
+1015c1088,1097
 <     148	THROW(MyException)
 ---
 >     ?	STORE_LOCAL(value ex6)
@@ -296,25 +296,25 @@
 >     154	LOAD_LOCAL(value x4)
 >     154	IS_INSTANCE REF(class MyException)
 >     154	CZJUMP (BOOL)NE ? 5 : 8
-1042,1044d1123
+1036,1038d1117
 <     145	JUMP 4
 <     
 <   4: 
-1054,1057d1132
+1048,1051d1126
 <     154	LOAD_LOCAL(value x5)
 <     154	CALL_METHOD MyException.message (dynamic)
 <     154	STORE_LOCAL(value message)
 <     154	SCOPE_ENTER value message
-1059c1134,1135
+1053c1128,1129
 <     154	LOAD_LOCAL(value message)
 ---
 >     ?	LOAD_LOCAL(value x5)
 >     154	CALL_METHOD MyException.message (dynamic)
-1276c1352
+1270c1346
 <   blocks: [1,2,3,4,5,7]
 ---
 >   blocks: [1,2,3,4,5,7,8]
-1300c1376,1383
+1294c1370,1377
 <     38	THROW(IllegalArgumentException)
 ---
 >     ?	STORE_LOCAL(value e)
@@ -325,20 +325,20 @@
 >     42	CONSTANT("IllegalArgumentException")
 >     42	CALL_METHOD scala.Predef.println (dynamic)
 >     42	JUMP 2
-1347c1430
+1341c1424
 <   locals: value args, variable result, value ex6, value x4, value x5, value message, value x
 ---
 >   locals: value args, variable result, value ex6, value x4, value x5, value x
-1349c1432
+1343c1426
 <   blocks: [1,2,3,4,5,8,10,11,13,14,16]
 ---
 >   blocks: [1,2,3,5,8,10,11,13,14,16,17]
-1373c1456,1457
+1367c1450,1451
 <     203	THROW(MyException)
 ---
 >     ?	STORE_LOCAL(value ex6)
 >     ?	JUMP 17
-1393c1477,1486
+1387c1471,1480
 <     209	THROW(MyException)
 ---
 >     ?	STORE_LOCAL(value ex6)
@@ -351,41 +351,41 @@
 >     212	LOAD_LOCAL(value x4)
 >     212	IS_INSTANCE REF(class MyException)
 >     212	CZJUMP (BOOL)NE ? 5 : 8
-1406,1408d1498
+1400,1402d1492
 <     200	JUMP 4
 <     
 <   4: 
-1418,1421d1507
+1412,1415d1501
 <     212	LOAD_LOCAL(value x5)
 <     212	CALL_METHOD MyException.message (dynamic)
 <     212	STORE_LOCAL(value message)
 <     212	SCOPE_ENTER value message
-1423c1509,1510
+1417c1503,1504
 <     213	LOAD_LOCAL(value message)
 ---
 >     ?	LOAD_LOCAL(value x5)
 >     213	CALL_METHOD MyException.message (dynamic)
-1467c1554
+1461c1548
 <   blocks: [1,2,3,4,5,7]
 ---
 >   blocks: [1,2,3,4,5,7,8]
-1491c1578,1579
+1485c1572,1573
 <     58	THROW(IllegalArgumentException)
 ---
 >     ?	STORE_LOCAL(value e)
 >     ?	JUMP 8
-1492a1581,1586
+1486a1575,1580
 >   8: 
 >     62	LOAD_MODULE object Predef
 >     62	CONSTANT("RuntimeException")
 >     62	CALL_METHOD scala.Predef.println (dynamic)
 >     62	JUMP 2
 >     
-1540c1634
+1534c1628
 <   blocks: [1,2,3,4]
 ---
 >   blocks: [1,2,3,4,5]
-1560c1654,1659
+1554c1648,1653
 <     229	THROW(MyException)
 ---
 >     ?	JUMP 5
@@ -394,19 +394,19 @@
 >     ?	LOAD_LOCAL(variable monitor1)
 >     228	MONITOR_EXIT
 >     228	THROW(Throwable)
-1566c1665
+1560c1659
 <     ?	THROW(Throwable)
 ---
 >     228	THROW(Throwable)
-1594c1693
+1588c1687
 <   locals: value args, variable result, variable monitor2, variable monitorResult1
 ---
 >   locals: value exception$1, value args, variable result, variable monitor2, variable monitorResult1
-1596c1695
+1590c1689
 <   blocks: [1,2,3,4]
 ---
 >   blocks: [1,2,3,4,5]
-1619c1718,1726
+1613c1712,1720
 <     245	THROW(MyException)
 ---
 >     ?	STORE_LOCAL(value exception$1)
@@ -418,7 +418,7 @@
 >     ?	LOAD_LOCAL(variable monitor2)
 >     244	MONITOR_EXIT
 >     244	THROW(Throwable)
-1625c1732
+1619c1726
 <     ?	THROW(Throwable)
 ---
 >     244	THROW(Throwable)
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)
+}