Merge pull request #4413 from lrytz/opt/inliningEverything

Fixes and Improvements for the new inliner

15 files changed, 115 insertions, 31 deletions

diff --git a/test/files/neg/case-collision2.flags b/test/files/neg/case-collision2.flags
index 5bfa9da5c5..bea46902c9 100644
--- a/test/files/neg/case-collision2.flags
+++ b/test/files/neg/case-collision2.flags
@@ -1 +1 @@
--Ynooptimize -Ybackend:GenBCode -Xfatal-warnings
+-Ybackend:GenBCode -Xfatal-warnings
diff --git a/test/files/neg/inlineMaxSize.check b/test/files/neg/inlineMaxSize.check
new file mode 100644
index 0000000000..d218a8b6e2
--- /dev/null
+++ b/test/files/neg/inlineMaxSize.check
@@ -0,0 +1,9 @@
+inlineMaxSize.scala:7: warning: C::i()I is annotated @inline but could not be inlined:
+The size of the callsite method C::j()I
+would exceed the JVM method size limit after inlining C::i()I.
+
+  @inline final def j = i + i
+                            ^
+error: No warnings can be incurred under -Xfatal-warnings.
+one warning found
+one error found
diff --git a/test/files/neg/inlineMaxSize.flags b/test/files/neg/inlineMaxSize.flags
new file mode 100644
index 0000000000..9c6b811622
--- /dev/null
+++ b/test/files/neg/inlineMaxSize.flags
@@ -0,0 +1 @@
+-Ybackend:GenBCode -Ydelambdafy:method -Yopt:l:classpath -Yopt-warnings -Xfatal-warnings
\ No newline at end of file
diff --git a/test/files/neg/inlineMaxSize.scala b/test/files/neg/inlineMaxSize.scala
new file mode 100644
index 0000000000..16dc0d9538
--- /dev/null
+++ b/test/files/neg/inlineMaxSize.scala
@@ -0,0 +1,8 @@
+// not a JUnit test because of https://github.com/scala-opt/scala/issues/23
+class C {
+  @inline final def f = 0
+  @inline final def g = f + f + f + f + f + f + f + f + f + f
+  @inline final def h = g + g + g + g + g + g + g + g + g + g
+  @inline final def i = h + h + h + h + h + h + h + h + h + h
+  @inline final def j = i + i
+}
diff --git a/test/files/run/t7407.flags b/test/files/run/t7407.flags
index be4ef0798a..ffc65f4b81 100644
--- a/test/files/run/t7407.flags
+++ b/test/files/run/t7407.flags
@@ -1 +1 @@
--Ynooptimise -Yopt:l:none -Ybackend:GenBCode
+-Yopt:l:none -Ybackend:GenBCode
diff --git a/test/files/run/t7407b.flags b/test/files/run/t7407b.flags
index c8547a27dc..c30091d3de 100644
--- a/test/files/run/t7407b.flags
+++ b/test/files/run/t7407b.flags
@@ -1 +1 @@
--Ynooptimise -Ybackend:GenBCode
+-Ybackend:GenBCode
diff --git a/test/files/run/t8845.flags b/test/files/run/t8845.flags
index aada25f80d..c30091d3de 100644
--- a/test/files/run/t8845.flags
+++ b/test/files/run/t8845.flags
@@ -1 +1 @@
--Ybackend:GenBCode -Ynooptimize
+-Ybackend:GenBCode
diff --git a/test/files/run/t8925.flags b/test/files/run/t8925.flags
index be4ef0798a..ffc65f4b81 100644
--- a/test/files/run/t8925.flags
+++ b/test/files/run/t8925.flags
@@ -1 +1 @@
--Ynooptimise -Yopt:l:none -Ybackend:GenBCode
+-Yopt:l:none -Ybackend:GenBCode
diff --git a/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala b/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala
index 5d5215d887..d0ffd06b01 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala
@@ -8,7 +8,6 @@ import scala.reflect.io.VirtualDirectory
 import scala.tools.asm.Opcodes
 import scala.tools.asm.tree.{ClassNode, MethodNode}
 import scala.tools.cmd.CommandLineParser
-import scala.tools.nsc.backend.jvm.opt.LocalOpt
 import scala.tools.nsc.io.AbstractFile
 import scala.tools.nsc.reporters.StoreReporter
 import scala.tools.nsc.settings.MutableSettings
@@ -157,12 +156,6 @@ object CodeGenTools {
     assertTrue(h.start == insVec(startIndex) && h.end == insVec(endIndex) && h.handler == insVec(handlerIndex))
   }
 
-  val localOpt = {
-    val settings = new MutableSettings(msg => throw new IllegalArgumentException(msg))
-    settings.processArguments(List("-Yopt:l:method"), processAll = true)
-    new LocalOpt(settings)
-  }
-
   import scala.language.implicitConversions
 
   implicit def aliveInstruction(ins: Instruction): (Instruction, Boolean) = (ins, true)
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyExceptionHandlersTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyExceptionHandlersTest.scala
index 7b0504fec0..cb01f3d164 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyExceptionHandlersTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyExceptionHandlersTest.scala
@@ -40,7 +40,7 @@ class EmptyExceptionHandlersTest extends ClearAfterClass {
       Op(RETURN)
     )
     assertTrue(convertMethod(asmMethod).handlers.length == 1)
-    localOpt.removeEmptyExceptionHandlers(asmMethod)
+    LocalOptImpls.removeEmptyExceptionHandlers(asmMethod)
     assertTrue(convertMethod(asmMethod).handlers.isEmpty)
   }
 
@@ -61,7 +61,7 @@ class EmptyExceptionHandlersTest extends ClearAfterClass {
       Op(RETURN)
     )
     assertTrue(convertMethod(asmMethod).handlers.length == 1)
-    localOpt.removeEmptyExceptionHandlers(asmMethod)
+    LocalOptImpls.removeEmptyExceptionHandlers(asmMethod)
     assertTrue(convertMethod(asmMethod).handlers.isEmpty)
   }
 
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyLabelsAndLineNumbersTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyLabelsAndLineNumbersTest.scala
index 8c0168826e..7283e20745 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyLabelsAndLineNumbersTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyLabelsAndLineNumbersTest.scala
@@ -42,14 +42,14 @@ class EmptyLabelsAndLineNumbersTest {
     )
 
     val method = genMethod()(ops.map(_._1): _*)
-    assertTrue(localOpt.removeEmptyLineNumbers(method))
+    assertTrue(LocalOptImpls.removeEmptyLineNumbers(method))
     assertSameCode(instructionsFromMethod(method), ops.filter(_._2).map(_._1))
   }
 
   @Test
   def badlyLocatedLineNumbers(): Unit = {
     def t(ops: Instruction*) =
-      assertThrows[AssertionError](localOpt.removeEmptyLineNumbers(genMethod()(ops: _*)))
+      assertThrows[AssertionError](LocalOptImpls.removeEmptyLineNumbers(genMethod()(ops: _*)))
 
     // line numbers have to be right after their referenced label node
     t(LineNumber(0, Label(1)), Label(1))
@@ -88,7 +88,7 @@ class EmptyLabelsAndLineNumbersTest {
     )
 
     val method = genMethod(handlers = handler)(ops(2, 3, 8, 8, 9, 11).map(_._1): _*)
-    assertTrue(localOpt.removeEmptyLabelNodes(method))
+    assertTrue(LocalOptImpls.removeEmptyLabelNodes(method))
     val m = convertMethod(method)
     assertSameCode(m.instructions, ops(1, 1, 7, 7, 7, 10).filter(_._2).map(_._1))
     assertTrue(m.handlers match {
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala
index fedc074a15..029caa995c 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala
@@ -143,4 +143,52 @@ class InlineWarningTest extends ClearAfterClass {
     compileClasses(newCompiler(extraArgs = InlineWarningTest.argsNoWarn + " -Yopt-warnings:no-inline-mixed"))(scalaCode, List((javaCode, "A.java")), allowMessage = i => {c += 1; warns.exists(i.msg contains _)})
     assert(c == 2, c)
   }
+
+  @Test
+  def cannotInlinePrivateCallIntoDifferentClass(): Unit = {
+    val code =
+      """class M {
+        |  @inline final def f = {
+        |    @noinline def nested = 0
+        |    nested
+        |  }
+        |
+        |  def t = f // ok
+        |}
+        |
+        |class N {
+        |  def t(a: M) = a.f // not possible
+        |}
+      """.stripMargin
+
+    val warn =
+      """M::f()I is annotated @inline but could not be inlined:
+        |The callee M::f()I contains the instruction INVOKESPECIAL M.nested$1 ()I
+        |that would cause an IllegalAccessError when inlined into class N""".stripMargin
+
+    var c = 0
+    compile(code, allowMessage = i => { c += 1; i.msg contains warn })
+    assert(c == 1, c)
+  }
+
+  @Test
+  def cannotMixStrictfp(): Unit = {
+    val code =
+      """import annotation.strictfp
+        |class C {
+        |  @strictfp @inline final def f = 0
+        |  @strictfp def t1 = f
+        |  def t2 = f
+        |}
+      """.stripMargin
+
+    val warn =
+      """C::f()I is annotated @inline but could not be inlined:
+        |The callsite method C::t2()I
+        |does not have the same strictfp mode as the callee C::f()I.""".stripMargin
+
+    var c = 0
+    compile(code, allowMessage = i => { c += 1; i.msg contains warn })
+    assert(c == 1, c)
+  }
 }
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
index 39fb28570e..17724aecb1 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
@@ -152,7 +152,7 @@ class InlinerTest extends ClearAfterClass {
 
     assertSameCode(convertMethod(g).instructions.dropNonOp.take(4), expectedInlined)
 
-    localOpt.methodOptimizations(g, "C")
+    compiler.genBCode.bTypes.localOpt.methodOptimizations(g, "C")
     assertSameCode(convertMethod(g).instructions.dropNonOp,
       expectedInlined ++ List(VarOp(ASTORE, 2), VarOp(ALOAD, 2), Op(ATHROW)))
   }
@@ -950,4 +950,29 @@ class InlinerTest extends ClearAfterClass {
     assertInvoke(getSingleMethod(t, "t3"), "B", "<init>")
     assertInvoke(getSingleMethod(t, "t4"), "B", "<init>")
   }
+
+  @Test
+  def dontInlineNative(): Unit = {
+    val code =
+      """class C {
+        |  def t = System.arraycopy(null, 0, null, 0, 0)
+        |}
+      """.stripMargin
+    val List(c) = compileClasses(newCompiler(extraArgs = InlinerTest.args + " -Yopt-inline-heuristics:everything"))(code)
+    assertInvoke(getSingleMethod(c, "t"), "java/lang/System", "arraycopy")
+  }
+
+  @Test
+  def inlineMayRenderCodeDead(): Unit = {
+    val code =
+      """class C {
+        |  @inline final def f: String = throw new Error("")
+        |  @inline final def g: String = "a" + f + "b"       // after inlining f, need to run DCE, because the rest of g becomes dead.
+        |  def t = g                                         // the inliner requires no dead code when inlining g (uses an Analyzer).
+        |}
+      """.stripMargin
+
+    val List(c) = compile(code)
+    assertInvoke(getSingleMethod(c, "t"), "java/lang/Error", "<init>")
+  }
 }
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/SimplifyJumpsTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/SimplifyJumpsTest.scala
index 360fa1d23d..a685ae7dd5 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/SimplifyJumpsTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/SimplifyJumpsTest.scala
@@ -26,7 +26,7 @@ class SimplifyJumpsTest {
       Op(RETURN)
     )
     val method = genMethod()(ops: _*)
-    assertTrue(localOpt.simplifyJumps(method))
+    assertTrue(LocalOptImpls.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), Op(RETURN) :: ops.tail)
   }
 
@@ -45,7 +45,7 @@ class SimplifyJumpsTest {
       Jump(GOTO, Label(2)) :: // replaced by ATHROW
       rest: _*
     )
-    assertTrue(localOpt.simplifyJumps(method))
+    assertTrue(LocalOptImpls.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), Op(ACONST_NULL) :: Op(ATHROW) :: rest)
   }
 
@@ -66,11 +66,11 @@ class SimplifyJumpsTest {
       Op(RETURN)
     )
     val method = genMethod(handlers = handler)(initialInstrs: _*)
-    assertFalse(localOpt.simplifyJumps(method))
+    assertFalse(LocalOptImpls.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), initialInstrs)
 
     val optMethod = genMethod()(initialInstrs: _*) // no handler
-    assertTrue(localOpt.simplifyJumps(optMethod))
+    assertTrue(LocalOptImpls.simplifyJumps(optMethod))
     assertSameCode(instructionsFromMethod(optMethod).take(3), List(Label(1), Op(ACONST_NULL), Op(ATHROW)))
   }
 
@@ -91,7 +91,7 @@ class SimplifyJumpsTest {
       Op(IRETURN)
     )
     val method = genMethod()(begin ::: rest: _*)
-    assertTrue(localOpt.simplifyJumps(method))
+    assertTrue(LocalOptImpls.simplifyJumps(method))
     assertSameCode(
       instructionsFromMethod(method),
       List(VarOp(ILOAD, 1), Jump(IFLT, Label(3))) ::: rest.tail )
@@ -99,7 +99,7 @@ class SimplifyJumpsTest {
     // no label allowed between begin and rest. if there's another label, then there could be a
     // branch that label. eliminating the  GOTO would change the behavior.
     val nonOptMethod = genMethod()(begin ::: Label(22) :: rest: _*)
-    assertFalse(localOpt.simplifyJumps(nonOptMethod))
+    assertFalse(LocalOptImpls.simplifyJumps(nonOptMethod))
   }
 
   @Test
@@ -116,7 +116,7 @@ class SimplifyJumpsTest {
     // ensures that the goto is safely removed. ASM supports removing while iterating, but not the
     // next element of the current. Here, the current is the IFGE, the next is the GOTO.
     val method = genMethod()(code(Jump(IFGE, Label(2)), Jump(GOTO, Label(3))): _*)
-    assertTrue(localOpt.simplifyJumps(method))
+    assertTrue(LocalOptImpls.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), code(Jump(IFLT, Label(3))))
   }
 
@@ -131,7 +131,7 @@ class SimplifyJumpsTest {
       Op(IRETURN)
     )
     val method = genMethod()(ops: _*)
-    assertTrue(localOpt.simplifyJumps(method))
+    assertTrue(LocalOptImpls.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), ops.tail)
   }
 
@@ -157,7 +157,7 @@ class SimplifyJumpsTest {
       Op(IRETURN)
     )
     val method = genMethod()(ops(1, 2, 3): _*)
-    assertTrue(localOpt.simplifyJumps(method))
+    assertTrue(LocalOptImpls.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), ops(3, 3, 3))
   }
 
@@ -181,7 +181,7 @@ class SimplifyJumpsTest {
     )
 
     val method = genMethod()(ops(2): _*)
-    assertTrue(localOpt.simplifyJumps(method))
+    assertTrue(LocalOptImpls.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), ops(3))
   }
 
@@ -202,7 +202,7 @@ class SimplifyJumpsTest {
     )
 
     val method = genMethod()(ops(Jump(IFGE, Label(1))): _*)
-    assertTrue(localOpt.simplifyJumps(method))
+    assertTrue(LocalOptImpls.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), ops(Op(POP)))
   }
 
@@ -215,7 +215,7 @@ class SimplifyJumpsTest {
       Jump(GOTO, Label(1))
     )
     val method = genMethod()(ops(List(Jump(IF_ICMPGE, Label(1)))): _*)
-    assertTrue(localOpt.simplifyJumps(method))
+    assertTrue(LocalOptImpls.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), ops(List(Op(POP), Op(POP))))
   }
 }
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 da9853148b..902af7b7fa 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala
@@ -44,7 +44,7 @@ class UnreachableCodeTest extends ClearAfterClass {
 
   def assertEliminateDead(code: (Instruction, Boolean)*): Unit = {
     val method = genMethod()(code.map(_._1): _*)
-    localOpt.removeUnreachableCodeImpl(method, "C")
+    LocalOptImpls.removeUnreachableCodeImpl(method, "C")
     val nonEliminated = instructionsFromMethod(method)
     val expectedLive = code.filter(_._2).map(_._1).toList
     assertSameCode(nonEliminated, expectedLive)