Enable nullness analysis in the inliner

When inlining an instance call, the inliner has to ensure that a NPE
is still thrown if the receiver object is null. By using the nullness
analysis, we can avoid emitting this code in case the receiver object
is known to be not-null.

5 files changed, 43 insertions, 15 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala
index 028f0f8fa6..c6df86b297 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala
@@ -8,12 +8,14 @@ package backend.jvm
 package opt
 
 import scala.reflect.internal.util.{NoPosition, Position}
+import scala.tools.asm.{Opcodes, Type}
 import scala.tools.asm.tree._
 import scala.collection.convert.decorateAsScala._
-import scala.tools.nsc.backend.jvm.BTypes.{MethodInlineInfo, InternalName}
+import scala.tools.nsc.backend.jvm.BTypes.InternalName
 import scala.tools.nsc.backend.jvm.BackendReporting._
-import scala.tools.nsc.backend.jvm.opt.BytecodeUtils.AsmAnalyzer
+import scala.tools.nsc.backend.jvm.analysis.{NotNull, NullnessAnalyzer}
 import ByteCodeRepository.{Source, CompilationUnit}
+import BytecodeUtils._
 
 class CallGraph[BT <: BTypes](val btypes: BT) {
   import btypes._
@@ -93,12 +95,13 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
     // TODO: run dataflow analyses to make the call graph more precise
     //  - producers to get forwarded parameters (ForwardedParam)
     //  - typeAnalysis for more precise argument types, more precise callee
-    //  - nullAnalysis to skip emitting the receiver-null-check when inlining
 
-    // TODO: for now we run a basic analyzer to get the stack height at the call site.
-    // once we run a more elaborate analyzer (types, nullness), we can get the stack height out of there.
+    // For now we run a NullnessAnalyzer. It is used to determine if the receiver of an instance
+    // call is known to be not-null, in which case we don't have to emit a null check when inlining.
+    // It is also used to get the stack height at the call site.
     localOpt.minimalRemoveUnreachableCode(methodNode, definingClass.internalName)
-    val analyzer = new AsmAnalyzer(methodNode, definingClass.internalName)
+    val analyzer = new NullnessAnalyzer
+    analyzer.analyze(definingClass.internalName, methodNode)
 
     methodNode.instructions.iterator.asScala.collect({
       case call: MethodInsnNode =>
@@ -126,13 +129,20 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
           Nil
         }
 
+        val receiverNotNull = call.getOpcode == Opcodes.INVOKESTATIC || {
+          val numArgs = Type.getArgumentTypes(call.desc).length
+          val frame = analyzer.frameAt(call, methodNode)
+          frame.getStack(frame.getStackSize - 1 - numArgs).nullness == NotNull
+        }
+
         Callsite(
           callsiteInstruction = call,
           callsiteMethod = methodNode,
           callsiteClass = definingClass,
           callee = callee,
           argInfos = argInfos,
-          callsiteStackHeight = analyzer.frameAt(call).getStackSize,
+          callsiteStackHeight = analyzer.frameAt(call, methodNode).getStackSize,
+          receiverKnownNotNull = receiverNotNull,
           callsitePosition = callsitePositions.getOrElse(call, NoPosition)
         )
     }).toList
@@ -154,7 +164,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
    */
   final case class Callsite(callsiteInstruction: MethodInsnNode, callsiteMethod: MethodNode, callsiteClass: ClassBType,
                             callee: Either[OptimizerWarning, Callee], argInfos: List[ArgInfo],
-                            callsiteStackHeight: Int, callsitePosition: Position) {
+                            callsiteStackHeight: Int, receiverKnownNotNull: Boolean, callsitePosition: Position) {
     override def toString =
       "Invocation of" +
         s" ${callee.map(_.calleeDeclarationClass.internalName).getOrElse("?")}.${callsiteInstruction.name + callsiteInstruction.desc}" +
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala
index 3aca15da69..814c78b69c 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala
@@ -49,7 +49,7 @@ class Inliner[BT <: BTypes](val btypes: BT) {
       if (callGraph.callsites contains request.callsiteInstruction) {
         val r = inline(request.callsiteInstruction, request.callsiteStackHeight, request.callsiteMethod, request.callsiteClass,
           callee.callee, callee.calleeDeclarationClass,
-          receiverKnownNotNull = false, keepLineNumbers = false)
+          request.receiverKnownNotNull, keepLineNumbers = false)
 
         for (warning <- r) {
           if ((callee.annotatedInline && btypes.compilerSettings.YoptWarningEmitAtInlineFailed) || warning.emitWarning(compilerSettings)) {
@@ -89,7 +89,7 @@ class Inliner[BT <: BTypes](val btypes: BT) {
    */
   def selectCallsitesForInlining: List[Callsite] = {
     callsites.valuesIterator.filter({
-      case callsite @ Callsite(_, _, _, Right(Callee(callee, calleeDeclClass, safeToInline, _, annotatedInline, _, warning)), _, _, pos) =>
+      case callsite @ Callsite(_, _, _, Right(Callee(callee, calleeDeclClass, safeToInline, _, annotatedInline, _, warning)), _, _, _, pos) =>
         val res = doInlineCallsite(callsite)
 
         if (!res) {
@@ -112,7 +112,7 @@ class Inliner[BT <: BTypes](val btypes: BT) {
 
         res
 
-      case Callsite(ins, _, _, Left(warning), _, _, pos) =>
+      case Callsite(ins, _, _, Left(warning), _, _, _, pos) =>
         if (warning.emitWarning(compilerSettings))
           backendReporting.inlinerWarning(pos, s"failed to determine if ${ins.name} should be inlined:\n$warning")
         false
@@ -123,7 +123,7 @@ class Inliner[BT <: BTypes](val btypes: BT) {
    * The current inlining heuristics are simple: inline calls to methods annotated @inline.
    */
   def doInlineCallsite(callsite: Callsite): Boolean = callsite match {
-    case Callsite(_, _, _, Right(Callee(callee, calleeDeclClass, safeToInline, _, annotatedInline, _, warning)), _, _, pos) =>
+    case Callsite(_, _, _, Right(Callee(callee, calleeDeclClass, safeToInline, _, annotatedInline, _, warning)), _, _, _, pos) =>
       if (compilerSettings.YoptInlineHeuristics.value == "everything") safeToInline
       else annotatedInline && safeToInline
 
@@ -215,6 +215,7 @@ class Inliner[BT <: BTypes](val btypes: BT) {
             calleeInfoWarning      = infoWarning)),
           argInfos            = Nil,
           callsiteStackHeight = callsite.callsiteStackHeight,
+          receiverKnownNotNull = callsite.receiverKnownNotNull,
           callsitePosition = callsite.callsitePosition
         )
         callGraph.callsites(newCallsiteInstruction) = staticCallsite
@@ -444,6 +445,7 @@ class Inliner[BT <: BTypes](val btypes: BT) {
                 callee = originalCallsite.callee,
                 argInfos = Nil, // TODO: re-compute argInfos for new destination (once we actually compute them)
                 callsiteStackHeight = callsiteStackHeight + originalCallsite.callsiteStackHeight,
+                receiverKnownNotNull = originalCallsite.receiverKnownNotNull,
                 callsitePosition = originalCallsite.callsitePosition
               )
 
diff --git a/test/files/neg/inlineMaxSize.check b/test/files/neg/inlineMaxSize.check
index d218a8b6e2..9d790e154c 100644
--- a/test/files/neg/inlineMaxSize.check
+++ b/test/files/neg/inlineMaxSize.check
@@ -2,8 +2,8 @@ inlineMaxSize.scala:7: warning: C::i()I is annotated @inline but could not be in
 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
-                            ^
+  @inline final def j = i + i + i
+                                ^
 error: No warnings can be incurred under -Xfatal-warnings.
 one warning found
 one error found
diff --git a/test/files/neg/inlineMaxSize.scala b/test/files/neg/inlineMaxSize.scala
index 16dc0d9538..9d2db1a357 100644
--- a/test/files/neg/inlineMaxSize.scala
+++ b/test/files/neg/inlineMaxSize.scala
@@ -4,5 +4,5 @@ class C {
   @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
+  @inline final def j = i + i + i
 }
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 0fc3601603..b8c5f85c49 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
@@ -975,4 +975,20 @@ class InlinerTest extends ClearAfterClass {
     val List(c) = compile(code)
     assertInvoke(getSingleMethod(c, "t"), "java/lang/Error", "<init>")
   }
+
+  @Test
+  def noRedunantNullChecks(): Unit = {
+    val code =
+      """class C {
+        |  @inline final def f: String = "hai!"
+        |  def t(c: C) = {c.f; c.f} // null check on the first, but not the second
+        |}
+      """.stripMargin
+
+    val List(c) = compile(code)
+    val t = getSingleMethod(c, "t").instructions
+    assertNoInvoke(t)
+    assert(2 == t.collect({case Ldc(_, "hai!") => }).size)     // twice the body of f
+    assert(1 == t.collect({case Jump(IFNONNULL, _) => }).size) // one single null check
+  }
 }