Clarify why we emit ATHROW after expressions of type Nothing

Tests for emitting expressions of type Nothing.

9 files changed, 237 insertions, 6 deletions

diff --git a/test/files/run/nothingTypeDce.flags b/test/files/run/nothingTypeDce.flags
new file mode 100644
index 0000000000..d85321ca0e
--- /dev/null
+++ b/test/files/run/nothingTypeDce.flags
@@ -0,0 +1 @@
+-target:jvm-1.6 -Ybackend:GenBCode -Yopt:unreachable-code
diff --git a/test/files/run/nothingTypeDce.scala b/test/files/run/nothingTypeDce.scala
new file mode 100644
index 0000000000..4b9d934eac
--- /dev/null
+++ b/test/files/run/nothingTypeDce.scala
@@ -0,0 +1,61 @@
+// See comment in BCodeBodyBuilder
+
+// -target:jvm-1.6 -Ybackend:GenBCode -Yopt:unreachable-code
+// target enables stack map frames generation
+
+class C {
+  // can't just emit a call to ???, that returns value of type Nothing$ (not Int).
+  def f1: Int = ???
+
+  def f2: Int = throw new Error("")
+
+  def f3(x: Boolean) = {
+    var y = 0
+    // cannot assign an object of type Nothing$ to Int
+    if (x) y = ???
+    else   y = 1
+    y
+  }
+
+  def f4(x: Boolean) = {
+    var y = 0
+    // tests that whatever is emitted after the throw is valid (what? depends on opts, presence of stack map frames)
+    if (x) y = throw new Error("")
+    else   y = 1
+    y
+  }
+
+  def f5(x: Boolean) = {
+    // stack heights need to be the smae. ??? looks to the jvm like returning a value of
+    // type Nothing$, need to drop or throw it.
+    println(
+      if (x) { ???; 10 }
+      else 20
+    )
+  }
+
+  def f6(x: Boolean) = {
+    println(
+      if (x) { throw new Error(""); 10 }
+      else 20
+    )
+  }
+
+  def f7(x: Boolean) = {
+    println(
+      if (x) throw new Error("")
+      else 20
+    )
+  }
+
+  def f8(x: Boolean) = {
+    println(
+      if (x) throw new Error("")
+      else 20
+    )
+  }
+}
+
+object Test extends App {
+  new C()
+}
diff --git a/test/files/run/nothingTypeNoFramesNoDce.check b/test/files/run/nothingTypeNoFramesNoDce.check
new file mode 100644
index 0000000000..b1d08b45ff
--- /dev/null
+++ b/test/files/run/nothingTypeNoFramesNoDce.check
@@ -0,0 +1 @@
+warning: -target:jvm-1.5 is deprecated: use target for Java 1.6 or above.
diff --git a/test/files/run/nothingTypeNoFramesNoDce.flags b/test/files/run/nothingTypeNoFramesNoDce.flags
new file mode 100644
index 0000000000..a035c86179
--- /dev/null
+++ b/test/files/run/nothingTypeNoFramesNoDce.flags
@@ -0,0 +1 @@
+-target:jvm-1.5 -Ybackend:GenBCode -Yopt:l:none -deprecation
diff --git a/test/files/run/nothingTypeNoFramesNoDce.scala b/test/files/run/nothingTypeNoFramesNoDce.scala
new file mode 100644
index 0000000000..3d1298303a
--- /dev/null
+++ b/test/files/run/nothingTypeNoFramesNoDce.scala
@@ -0,0 +1,61 @@
+// See comment in BCodeBodyBuilder
+
+// -target:jvm-1.5 -Ybackend:GenBCode -Yopt:l:none
+// target disables stack map frame generation. in this mode, the ClssWriter just emits dead code as is.
+
+class C {
+  // can't just emit a call to ???, that returns value of type Nothing$ (not Int).
+  def f1: Int = ???
+
+  def f2: Int = throw new Error("")
+
+  def f3(x: Boolean) = {
+    var y = 0
+    // cannot assign an object of type Nothing$ to Int
+    if (x) y = ???
+    else   y = 1
+    y
+  }
+
+  def f4(x: Boolean) = {
+    var y = 0
+    // tests that whatever is emitted after the throw is valid (what? depends on opts, presence of stack map frames)
+    if (x) y = throw new Error("")
+    else   y = 1
+    y
+  }
+
+  def f5(x: Boolean) = {
+    // stack heights need to be the smae. ??? looks to the jvm like returning a value of
+    // type Nothing$, need to drop or throw it.
+    println(
+      if (x) { ???; 10 }
+      else 20
+    )
+  }
+
+  def f6(x: Boolean) = {
+    println(
+      if (x) { throw new Error(""); 10 }
+      else 20
+    )
+  }
+
+  def f7(x: Boolean) = {
+    println(
+      if (x) throw new Error("")
+      else 20
+    )
+  }
+
+  def f8(x: Boolean) = {
+    println(
+      if (x) throw new Error("")
+      else 20
+    )
+  }
+}
+
+object Test extends App {
+  new C()
+}
diff --git a/test/files/run/nothingTypeNoOpt.flags b/test/files/run/nothingTypeNoOpt.flags
new file mode 100644
index 0000000000..b3b518051b
--- /dev/null
+++ b/test/files/run/nothingTypeNoOpt.flags
@@ -0,0 +1 @@
+-target:jvm-1.6 -Ybackend:GenBCode -Yopt:l:none
diff --git a/test/files/run/nothingTypeNoOpt.scala b/test/files/run/nothingTypeNoOpt.scala
new file mode 100644
index 0000000000..5c5a20fa3b
--- /dev/null
+++ b/test/files/run/nothingTypeNoOpt.scala
@@ -0,0 +1,61 @@
+// See comment in BCodeBodyBuilder
+
+// -target:jvm-1.6 -Ybackend:GenBCode -Yopt:l:none
+// target enables stack map frame generation
+
+class C {
+  // can't just emit a call to ???, that returns value of type Nothing$ (not Int).
+  def f1: Int = ???
+
+  def f2: Int = throw new Error("")
+
+  def f3(x: Boolean) = {
+    var y = 0
+    // cannot assign an object of type Nothing$ to Int
+    if (x) y = ???
+    else   y = 1
+    y
+  }
+
+  def f4(x: Boolean) = {
+    var y = 0
+    // tests that whatever is emitted after the throw is valid (what? depends on opts, presence of stack map frames)
+    if (x) y = throw new Error("")
+    else   y = 1
+    y
+  }
+
+  def f5(x: Boolean) = {
+    // stack heights need to be the smae. ??? looks to the jvm like returning a value of
+    // type Nothing$, need to drop or throw it.
+    println(
+      if (x) { ???; 10 }
+      else 20
+    )
+  }
+
+  def f6(x: Boolean) = {
+    println(
+      if (x) { throw new Error(""); 10 }
+      else 20
+    )
+  }
+
+  def f7(x: Boolean) = {
+    println(
+      if (x) throw new Error("")
+      else 20
+    )
+  }
+
+  def f8(x: Boolean) = {
+    println(
+      if (x) throw new Error("")
+      else 20
+    )
+  }
+}
+
+object Test extends App {
+  new C()
+}
diff --git a/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala b/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala
index 8518d5c832..fb677fc84b 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala
@@ -42,7 +42,7 @@ object CodeGenTools {
     compiler
   }
 
-  def compile(compiler: Global = newCompiler())(code: String): List[(String, Array[Byte])] = {
+  def compile(compiler: Global)(code: String): List[(String, Array[Byte])] = {
     compiler.reporter.reset()
     resetOutput(compiler)
     val run = new compiler.Run()
@@ -51,11 +51,16 @@ object CodeGenTools {
     (for (f <- outDir.iterator if !f.isDirectory) yield (f.name, f.toByteArray)).toList
   }
 
-  def compileClasses(compiler: Global = newCompiler())(code: String): List[ClassNode] = {
+  def compileClasses(compiler: Global)(code: String): List[ClassNode] = {
     compile(compiler)(code).map(p => AsmUtils.readClass(p._2)).sortBy(_.name)
   }
 
-  def compileMethods(compiler: Global = newCompiler())(code: String): List[MethodNode] = {
+  def compileMethods(compiler: Global)(code: String): List[MethodNode] = {
     compileClasses(compiler)(s"class C { $code }").head.methods.asScala.toList.filterNot(_.name == "<init>")
   }
+
+  def singleMethodInstructions(compiler: Global)(code: String): List[Instruction] = {
+    val List(m) = compileMethods(compiler)(code)
+    instructionsFromMethod(m)
+  }
 }
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 4e62cc64e4..ae08d10b79 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala
@@ -18,6 +18,14 @@ import ASMConverters._
 class UnreachableCodeTest {
   import UnreachableCodeTest._
 
+  // jvm-1.6 enables emitting stack map frames, which impacts the code generation wrt dead basic blocks,
+  // see comment in BCodeBodyBuilder
+  val dceCompiler = newCompiler(extraArgs = "-target:jvm-1.6 -Ybackend:GenBCode -Yopt:unreachable-code")
+  val noOptCompiler = newCompiler(extraArgs = "-target:jvm-1.6 -Ybackend:GenBCode -Yopt:l:none")
+
+  // jvm-1.5 disables computing stack map frames, and it emits dead code as-is.
+  val noOptNoFramesCompiler = newCompiler(extraArgs = "-target:jvm-1.5 -Ybackend:GenBCode -Yopt:l:none")
+
   @Test
   def basicElimination(): Unit = {
     assertEliminateDead(
@@ -97,6 +105,36 @@ class UnreachableCodeTest {
   }
 
   @Test
+  def basicEliminationCompiler(): Unit = {
+    val code = "def f: Int = { return 1; 2 }"
+    val withDce = singleMethodInstructions(dceCompiler)(code)
+    assertSameCode(withDce.dropNonOp, List(Op(ICONST_1), Op(IRETURN)))
+
+    val noDce = singleMethodInstructions(noOptCompiler)(code)
+
+    // The emitted code is ICONST_1, IRETURN, ICONST_2, IRETURN. The latter two are dead.
+    //
+    // GenBCode puts the last IRETURN into a new basic block: it emits a label before the second
+    // IRETURN. This is an implementation detail, it may change; it affects the outcome of this test.
+    //
+    // During classfile writing with COMPUTE_FAMES (-target:jvm-1.6 or larger), the ClassfileWriter
+    // puts the ICONST_2 into a new basic block, because the preceding operation (IRETURN) ends
+    // the current block. We get something like
+    //
+    //   L1: ICONST_1; IRETURN
+    //   L2: ICONST_2            << dead
+    //   L3: IRETURN             << dead
+    //
+    // Finally, instructions in the dead basic blocks are replaced by ATHROW, as explained in
+    // a comment in BCodeBodyBuilder.
+    assertSameCode(noDce.dropNonOp, List(Op(ICONST_1), Op(IRETURN), Op(ATHROW), Op(ATHROW)))
+
+    // when NOT computing stack map frames, ASM's ClassWriter does not replace dead code by NOP/ATHROW
+    val noDceNoFrames = singleMethodInstructions(noOptNoFramesCompiler)(code)
+    assertSameCode(noDceNoFrames.dropNonOp, List(Op(ICONST_1), Op(IRETURN), Op(ICONST_2), Op(IRETURN)))
+  }
+
+  @Test
   def metaTest(): Unit = {
     assertEliminateDead() // no instructions
 
@@ -153,10 +191,11 @@ object UnreachableCodeTest {
     val cls = wrapInClass(genMethod()(code.map(_._1): _*))
     LocalOpt.removeUnreachableCode(cls)
     val nonEliminated = instructionsFromMethod(cls.methods.get(0))
-
     val expectedLive = code.filter(_._2).map(_._1).toList
-
-    assertTrue(s"\nExpected: $expectedLive\nActual  : $nonEliminated", nonEliminated === expectedLive)
+    assertSameCode(nonEliminated, expectedLive)
   }
 
+  def assertSameCode(actual: List[Instruction], expected: List[Instruction]): Unit = {
+    assertTrue(s"\nExpected: $expected\nActual  : $actual", actual === expected)
+  }
 }