Remove stale local variables and exception handlers after DCE

This is required for correctness of the generated bytecode. Exception
handlers and local variable descriptors specify code offset ranges.
These offsets have to exist, not be eliminated.

5 files changed, 215 insertions, 7 deletions

diff --git a/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala b/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala
index fb677fc84b..15bc1f427d 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala
@@ -1,5 +1,7 @@
 package scala.tools.nsc.backend.jvm
 
+import org.junit.Assert._
+
 import scala.reflect.internal.util.BatchSourceFile
 import scala.reflect.io.VirtualDirectory
 import scala.tools.asm.Opcodes
@@ -16,9 +18,11 @@ object CodeGenTools {
                  name: String = "m",
                  descriptor: String = "()V",
                  genericSignature: String = null,
-                 throwsExceptions: Array[String] = null)(body: Instruction*): MethodNode = {
+                 throwsExceptions: Array[String] = null,
+                 handlers: List[ExceptionHandler] = Nil,
+                 localVars: List[LocalVariable] = Nil)(body: Instruction*): MethodNode = {
     val node = new MethodNode(flags, name, descriptor, genericSignature, throwsExceptions)
-    applyToMethod(node, body.toList)
+    applyToMethod(node, Method(body.toList, handlers, localVars))
     node
   }
 
@@ -63,4 +67,13 @@ object CodeGenTools {
     val List(m) = compileMethods(compiler)(code)
     instructionsFromMethod(m)
   }
+
+  def singleMethod(compiler: Global)(code: String): Method = {
+    val List(m) = compileMethods(compiler)(code)
+    convertMethod(m)
+  }
+
+  def assertSameCode(actual: List[Instruction], expected: List[Instruction]): Unit = {
+    assertTrue(s"\nExpected: $expected\nActual  : $actual", actual === expected)
+  }
 }
diff --git a/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala b/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala
index 640f22fc47..2fb5bb8052 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala
@@ -58,7 +58,7 @@ class DirectCompileTest {
       List(IntOp(BIPUSH, 10), Op(IRETURN)))
 
     assertTrue(instructionsFromMethod(g).dropNonOp ===
-      List(VarOp(ALOAD, 0), Method(INVOKEVIRTUAL, "C", "f", "()I", false), Op(IRETURN)))
+      List(VarOp(ALOAD, 0), Invoke(INVOKEVIRTUAL, "C", "f", "()I", false), Op(IRETURN)))
   }
 
   @Test
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyExceptionHandlersTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyExceptionHandlersTest.scala
new file mode 100644
index 0000000000..57fa1a7b66
--- /dev/null
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyExceptionHandlersTest.scala
@@ -0,0 +1,92 @@
+package scala.tools.nsc
+package backend.jvm
+package opt
+
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.junit.Test
+import scala.tools.asm.Opcodes._
+import org.junit.Assert._
+
+import CodeGenTools._
+import scala.tools.partest.ASMConverters
+import ASMConverters._
+
+@RunWith(classOf[JUnit4])
+class EmptyExceptionHandlersTest {
+
+  val exceptionDescriptor = "java/lang/Exception"
+
+  @Test
+  def eliminateEmpty(): Unit = {
+    val handlers = List(ExceptionHandler(Label(1), Label(2), Label(2), Some(exceptionDescriptor)))
+    val asmMethod = genMethod(handlers = handlers)(
+      Label(1),
+      Label(2),
+      Op(RETURN)
+    )
+    assertTrue(convertMethod(asmMethod).handlers.length == 1)
+    LocalOpt.removeEmptyExceptionHandlers(asmMethod)
+    assertTrue(convertMethod(asmMethod).handlers.isEmpty)
+  }
+
+  @Test
+  def eliminateHandlersGuardingNops(): Unit = {
+    val handlers = List(ExceptionHandler(Label(1), Label(2), Label(2), Some(exceptionDescriptor)))
+    val asmMethod = genMethod(handlers = handlers)(
+      Label(1),          // nops only
+      Op(NOP),
+      Op(NOP),
+      Jump(GOTO, Label(3)),
+      Op(NOP),
+      Label(3),
+      Op(NOP),
+      Jump(GOTO, Label(4)),
+
+      Label(2),          // handler
+      Op(ACONST_NULL),
+      Op(ATHROW),
+
+      Label(4),          // return
+      Op(RETURN)
+    )
+    assertTrue(convertMethod(asmMethod).handlers.length == 1)
+    LocalOpt.removeEmptyExceptionHandlers(asmMethod)
+    assertTrue(convertMethod(asmMethod).handlers.isEmpty)
+  }
+
+  val noOptCompiler       = newCompiler(extraArgs = "-Ybackend:GenBCode -Yopt:l:none")
+  val dceCompiler = newCompiler(extraArgs = "-Ybackend:GenBCode -Yopt:unreachable-code")
+
+  @Test
+  def eliminateUnreachableHandler(): Unit = {
+    val code = "def f: Unit = try { } catch { case _: Exception => println(0) }; println(1)"
+
+    assertTrue(singleMethod(noOptCompiler)(code).handlers.length == 1)
+    val optMethod = singleMethod(dceCompiler)(code)
+    assertTrue(optMethod.handlers.isEmpty)
+
+    val code2 =
+      """def f: Unit = {
+        |  println(0)
+        |  return
+        |  try { throw new Exception("") } // removed by dce, so handler will be removed as well
+        |  catch { case _: Exception => println(1) }
+        |  println(2)
+        |}""".stripMargin
+
+    assertTrue(singleMethod(dceCompiler)(code2).handlers.isEmpty)
+  }
+
+  @Test
+  def keepAliveHandlers(): Unit = {
+    val code =
+      """def f: Int = {
+        |  println(0)
+        |  try { 1 }
+        |  catch { case _: Exception => 2 }
+        |}""".stripMargin
+
+    assertTrue(singleMethod(dceCompiler)(code).handlers.length == 1)
+  }
+}
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala
index ae08d10b79..a3bd7ae6fe 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala
@@ -135,6 +135,26 @@ class UnreachableCodeTest {
   }
 
   @Test
+  def eliminateDeadCatchBlocks(): Unit = {
+    val code = "def f: Int = { return 0; try { 1 } catch { case _: Exception => 2 } }"
+    assertSameCode(singleMethodInstructions(dceCompiler)(code).dropNonOp,
+                   List(Op(ICONST_0), Op(IRETURN)))
+
+    val code2 = "def f: Unit = { try { } catch { case _: Exception => () }; () }"
+    // DCE only removes dead basic blocks, but not NOPs, and also not useless jumps
+    assertSameCode(singleMethodInstructions(dceCompiler)(code2).dropNonOp,
+                   List(Op(NOP), Jump(GOTO, Label(33)), Label(33), Op(RETURN)))
+
+    val code3 = "def f: Unit = { try { } catch { case _: Exception => try { } catch { case _: Exception => () } }; () }"
+    assertSameCode(singleMethodInstructions(dceCompiler)(code3).dropNonOp,
+      List(Op(NOP), Jump(GOTO, Label(33)), Label(33), Op(RETURN)))
+
+    val code4 = "def f: Unit = { try { try { } catch { case _: Exception => () } } catch { case _: Exception => () }; () }"
+    assertSameCode(singleMethodInstructions(dceCompiler)(code4).dropNonOp,
+      List(Op(NOP), Jump(GOTO, Label(4)), Label(4), Jump(GOTO, Label(7)), Label(7), Op(RETURN)))
+  }
+
+  @Test // test the dce-testing tools
   def metaTest(): Unit = {
     assertEliminateDead() // no instructions
 
@@ -194,8 +214,4 @@ object UnreachableCodeTest {
     val expectedLive = code.filter(_._2).map(_._1).toList
     assertSameCode(nonEliminated, expectedLive)
   }
-
-  def assertSameCode(actual: List[Instruction], expected: List[Instruction]): Unit = {
-    assertTrue(s"\nExpected: $expected\nActual  : $actual", actual === expected)
-  }
 }
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/UnusedLocalVariablesTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/UnusedLocalVariablesTest.scala
new file mode 100644
index 0000000000..24a1f9d1c1
--- /dev/null
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/UnusedLocalVariablesTest.scala
@@ -0,0 +1,87 @@
+package scala.tools.nsc
+package backend.jvm
+package opt
+
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.junit.Test
+import scala.tools.asm.Opcodes._
+import org.junit.Assert._
+import scala.collection.JavaConverters._
+
+import CodeGenTools._
+import scala.tools.partest.ASMConverters
+import ASMConverters._
+
+@RunWith(classOf[JUnit4])
+class UnusedLocalVariablesTest {
+  val dceCompiler = newCompiler(extraArgs = "-Ybackend:GenBCode -Yopt:unreachable-code")
+
+  @Test
+  def removeUnusedVar(): Unit = {
+    val code = """def f(a: Long, b: String, c: Double): Unit = { return; var x = a; var y = x + 10 }"""
+    assertLocalVarCount(code, 4) // `this, a, b, c`
+
+    val code2 = """def f(): Unit = { var x = if (true) return else () }"""
+    assertLocalVarCount(code2, 1) // x is eliminated, constant folding in scalac removes the if
+
+    val code3 = """def f: Unit = return""" // paramless method
+    assertLocalVarCount(code3, 1) // this
+  }
+
+  @Test
+  def keepUsedVar(): Unit = {
+    val code = """def f(a: Long, b: String, c: Double): Unit = { val x = 10 + a; val y = x + 10 }"""
+    assertLocalVarCount(code, 6)
+
+    val code2 = """def f(a: Long): Unit = { var x = if (a == 0l) return else () }"""
+    assertLocalVarCount(code2, 3) // remains
+  }
+
+  @Test
+  def constructorLocals(): Unit = {
+    val code = """class C {
+                 |  def this(a: Int) = {
+                 |    this()
+                 |    throw new Exception("")
+                 |    val y = 0
+                 |  }
+                 |}
+                 |""".stripMargin
+    val cls = compileClasses(dceCompiler)(code).head
+    val m = convertMethod(cls.methods.asScala.toList.find(_.desc == "(I)V").get)
+    assertTrue(m.localVars.length == 2) // this, a, but not y
+
+
+    val code2 =
+      """class C {
+        |  {
+        |    throw new Exception("")
+        |    val a = 0
+        |  }
+        |}
+        |
+        |object C {
+        |  {
+        |    throw new Exception("")
+        |    val b = 1
+        |  }
+        |}
+      """.stripMargin
+
+    val clss2 = compileClasses(dceCompiler)(code2)
+    val cls2 = clss2.find(_.name == "C").get
+    val companion2 = clss2.find(_.name == "C$").get
+
+    val clsConstr = convertMethod(cls2.methods.asScala.toList.find(_.name == "<init>").get)
+    val companionConstr = convertMethod(companion2.methods.asScala.toList.find(_.name == "<init>").get)
+
+    assertTrue(clsConstr.localVars.length == 1) // this
+    assertTrue(companionConstr.localVars.length == 1) // this
+  }
+
+  def assertLocalVarCount(code: String, numVars: Int): Unit = {
+    assertTrue(singleMethod(dceCompiler)(code).localVars.length == numVars)
+  }
+
+}