SI-9671, SI-7397 fix null.asInstanceOf[Int] when pt erases to Object

Erasure first replaces null.asInstanceOf[Int] by unbox(null). If the
expected type erases to object, erasure then introduces a box operation,
yielding box(unbox(null)). Note that this value is a box of zero, not
null.

Erasure has an optimization to replace box(unbox(x)) in case x is
of primitive type. 60f1b4b extended this to the case when x is null,
which is incorrect in general. The reason was to prevent creating a
primitive box to be stored in the unused generic field when creating
an instance of a specialized class. A special case ensures that this
optimization is still performed.

6 files changed, 145 insertions, 30 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala
index bc614dfc31..7bfe5a4740 100644
--- a/src/compiler/scala/tools/nsc/transform/Erasure.scala
+++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala
@@ -606,10 +606,8 @@ abstract class Erasure extends AddInterfaces
           // !!! Make pending/run/t5866b.scala work. The fix might be here and/or in unbox1.
           if (isPrimitiveValueType(targ.tpe) || isErasedValueType(targ.tpe)) {
             val noNullCheckNeeded = targ.tpe match {
-              case ErasedValueType(_, underlying) =>
-                isPrimitiveValueClass(underlying.typeSymbol)
-              case _ =>
-                true
+              case ErasedValueType(_, underlying) => isPrimitiveValueType(underlying)
+              case _ => true
             }
             if (noNullCheckNeeded) unbox(qual1, targ.tpe)
             else {
diff --git a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala
index 4b1f1efee4..e894c58b1a 100644
--- a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala
+++ b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala
@@ -1911,8 +1911,7 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers {
 
   /** Forward to the generic class constructor. If the current class initializes
    *  specialized fields corresponding to parameters, it passes null to the superclass
-   *  constructor. This saves the boxing cost for initializing generic fields that are
-   *  never used.
+   *  constructor.
    *
    *  For example:
    *  {{{
@@ -1926,7 +1925,17 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers {
    *        super.this(null.asInstanceOf[Int], null.asInstanceOf[Int])
    *      }
    *    }
-   *  }}
+   *  }}}
+   *
+   *  Note that erasure first transforms `null.asInstanceOf[Int]` to `unbox(null)`, which is 0.
+   *  Then it adapts the argument `unbox(null)` of type Int to the erased parameter type of Tuple2,
+   *  which is Object, so it inserts a `box` call and we get `box(unbox(null))`, which is
+   *  `new Integer(0)` (not `null`).
+   *
+   *  However it does not make sense to create an Integer instance to be stored in the generic field
+   *  of the superclass: that field is never used. Therefore we mark the `null` tree with the
+   *  [[SpecializedSuperConstructorCallArgument]] attachment and special-case erasure to replace
+   *  `box(unbox(null))` by `null` in this case.
    */
   private def forwardCtorCall(pos: scala.reflect.internal.util.Position, receiver: Tree, paramss: List[List[ValDef]], clazz: Symbol): Tree = {
     log(s"forwardCtorCall($pos, $receiver, $paramss, $clazz)")
@@ -1945,7 +1954,7 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers {
 
     val argss = mmap(paramss)(x =>
       if (initializesSpecializedField(x.symbol))
-        gen.mkAsInstanceOf(Literal(Constant(null)), x.symbol.tpe)
+        gen.mkAsInstanceOf(Literal(Constant(null)).updateAttachment(SpecializedSuperConstructorCallArgument), x.symbol.tpe)
       else
         Ident(x.symbol)
     )
@@ -1989,5 +1998,7 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers {
       }
 
       resultTree
-    }  }
+    }
+  }
+  object SpecializedSuperConstructorCallArgument
 }
diff --git a/src/compiler/scala/tools/nsc/transform/TypeAdaptingTransformer.scala b/src/compiler/scala/tools/nsc/transform/TypeAdaptingTransformer.scala
index afafdedce7..52d7c0b897 100644
--- a/src/compiler/scala/tools/nsc/transform/TypeAdaptingTransformer.scala
+++ b/src/compiler/scala/tools/nsc/transform/TypeAdaptingTransformer.scala
@@ -14,11 +14,18 @@ trait TypeAdaptingTransformer { self: TreeDSL =>
 
     def typedPos(pos: Position)(tree: Tree): Tree
 
+    /**
+     * SI-4148: can't always replace box(unbox(x)) by x because
+     *   - unboxing x may lead to throwing an exception, e.g. in "aah".asInstanceOf[Int]
+     *   - box(unbox(null)) is not `null` but the box of zero
+     */
     private def isSafelyRemovableUnbox(fn: Tree, arg: Tree): Boolean = {
-     currentRun.runDefinitions.isUnbox(fn.symbol) && {
-      val cls = arg.tpe.typeSymbol
-      (cls == NullClass) || isBoxedValueClass(cls)
-     }
+      currentRun.runDefinitions.isUnbox(fn.symbol) && {
+        // replace box(unbox(null)) by null when passed to the super constructor in a specialized
+        // class, see comment in SpecializeTypes.forwardCtorCall.
+        arg.hasAttachment[specializeTypes.SpecializedSuperConstructorCallArgument.type] ||
+          isBoxedValueClass(arg.tpe.typeSymbol)
+      }
     }
 
     private def isPrimitiveValueType(tpe: Type)      = isPrimitiveValueClass(tpe.typeSymbol)
@@ -44,14 +51,7 @@ trait TypeAdaptingTransformer { self: TreeDSL =>
             case x =>
               assert(x != ArrayClass)
               tree match {
-                /* Can't always remove a Box(Unbox(x)) combination because the process of boxing x
-                 * may lead to throwing an exception.
-                 *
-                 * This is important for specialization: calls to the super constructor should not box/unbox specialized
-                 * fields (see TupleX). (ID)
-                 */
                 case Apply(boxFun, List(arg)) if isSafelyRemovableUnbox(tree, arg) =>
-                  log(s"boxing an unbox: ${tree.symbol} -> ${arg.tpe}")
                   arg
                 case _ =>
                   (REF(currentRun.runDefinitions.boxMethod(x)) APPLY tree) setPos (tree.pos) setType ObjectTpe
diff --git a/test/junit/scala/BoxUnboxTest.scala b/test/junit/scala/BoxUnboxTest.scala
index 162d805a6b..eb7a35e98c 100644
--- a/test/junit/scala/BoxUnboxTest.scala
+++ b/test/junit/scala/BoxUnboxTest.scala
@@ -48,22 +48,22 @@ class BoxUnboxTest {
     assertEquals(n4, 0)
     val n5 = null.asInstanceOf[Int] == 0
     assertTrue(n5)
-    val n6 = null.asInstanceOf[Int] == null       // SI-9671 -- should be false, but is true
-    assertThrows[AssertionError](assertFalse(n6)) // should not throw
+    val n6 = null.asInstanceOf[Int] == null
+    assertFalse(n6)
     val n7 = null.asInstanceOf[Int] != 0
     assertFalse(n7)
-    val n8 = null.asInstanceOf[Int] != null      // SI-9671 -- should be true, but is false
-    assertThrows[AssertionError](assertTrue(n8)) // should not throw
+    val n8 = null.asInstanceOf[Int] != null
+    assertTrue(n8)
 
     val mp = new java.util.HashMap[Int, Int]
     val n9 = mp.get(0)
     assertEquals(n9, 0)
-    val n10 = mp.get(0) == null                    // SI-602 -- maybe related to SI-9671 (test above)?
+    val n10 = mp.get(0) == null                    // SI-602
     assertThrows[AssertionError](assertFalse(n10)) // should not throw
 
     def f(a: Any) = "" + a
-    val n11 = f(null.asInstanceOf[Int])                  // "null", should be "0". probably same cause as SI-602.
-    assertThrows[AssertionError](assertEquals(n11, "0")) // should not throw
+    val n11 = f(null.asInstanceOf[Int])
+    assertEquals(n11, "0")
 
     def n12 = genericNull[Int]
     assertEquals(n12, 0)
@@ -81,8 +81,8 @@ class BoxUnboxTest {
 
   @Test
   def boxUnboxBoolean(): Unit = {
-    val n1 = Option(null.asInstanceOf[Boolean])                 // SI-7397 -- should be Some(false), but is None
-    assertThrows[AssertionError](assertEquals(n1, Some(false))) // should not throw
+    val n1 = Option(null.asInstanceOf[Boolean])
+    assertEquals(n1, Some(false))
   }
 
   @Test
diff --git a/test/junit/scala/issues/BytecodeTest.scala b/test/junit/scala/issues/BytecodeTest.scala
index a720f20718..7b9474b52e 100644
--- a/test/junit/scala/issues/BytecodeTest.scala
+++ b/test/junit/scala/issues/BytecodeTest.scala
@@ -419,6 +419,16 @@ class BytecodeTest extends ClearAfterClass {
     assertInvoke(getSingleMethod(c, "f3"), "java/lang/Object", "hashCode")
     assertInvoke(getSingleMethod(c, "f4"), "java/lang/Object", "toString")
   }
+
+  @Test
+  def superConstructorArgumentInSpecializedClass(): Unit = {
+    // see comment in SpecializeTypes.forwardCtorCall
+    val code = "case class C[@specialized(Int) T](_1: T)"
+    val List(c, cMod, cSpec) = compileClasses(compiler)(code)
+    assertSameSummary(getSingleMethod(cSpec, "<init>"),
+      // pass `null` to super constructor, no box-unbox, no Integer created
+      List(ALOAD, ILOAD, PUTFIELD, ALOAD, ACONST_NULL, "<init>", RETURN))
+  }
 }
 
 object invocationReceiversTestCode {
diff --git a/test/junit/scala/issues/RunTest.scala b/test/junit/scala/issues/RunTest.scala
index 148009c912..b81a3e1d6f 100644
--- a/test/junit/scala/issues/RunTest.scala
+++ b/test/junit/scala/issues/RunTest.scala
@@ -10,8 +10,8 @@ import scala.tools.reflect.ToolBox
 import scala.tools.testing.ClearAfterClass
 
 object RunTest {
-
   class VC(val x: Any) extends AnyVal
+  class VCI(val x: Int) extends AnyVal { override def toString = "" + x }
 }
 
 @RunWith(classOf[JUnit4])
@@ -154,4 +154,100 @@ class RunTest extends ClearAfterClass {
     val u = Void.TYPE
     assertEquals(run[(Class[_], Class[_])](code), (u, u))
   }
+
+  @Test
+  def t9671(): Unit = {
+    val code =
+      """import scala.issues.RunTest.VCI
+        |
+        |def f1(a: Any) = "" + a
+        |def f2(a: AnyVal) = "" + a
+        |def f3[T](a: T) = "" + a
+        |def f4(a: Int) = "" + a
+        |def f5(a: VCI) = "" + a
+        |def f6(u: Unit) = "" + u
+        |
+        |def n1: AnyRef = null
+        |def n2: Null = null
+        |def n3: Any = null
+        |def n4[T]: T = null.asInstanceOf[T]
+        |
+        |def npe(s: => String) = try { s; throw new Error() } catch { case _: NullPointerException => "npe" }
+        |
+        |    f1(null.asInstanceOf[Int])  +
+        |    f1(  n1.asInstanceOf[Int])  +
+        |    f1(  n2.asInstanceOf[Int])  +
+        |    f1(  n3.asInstanceOf[Int])  +
+        |    f1(               n4[Int])  + // "null"
+        |"-"                             +
+        |    f1(null.asInstanceOf[VCI])  +
+        |npe(f1(  n1.asInstanceOf[VCI])) + // SI-8097
+        |    f1(  n2.asInstanceOf[VCI])  +
+        |npe(f1(  n3.asInstanceOf[VCI])) + // SI-8097
+        |    f1(               n4[VCI])  + // "null"
+        |"-"                             +
+        |    f1(null.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f1(  n1.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f1(  n2.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f1(  n3.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f1(               n4[Unit]) + // "null"
+        |"-"                             +
+        |    f2(null.asInstanceOf[Int])  +
+        |    f2(  n1.asInstanceOf[Int])  +
+        |    f2(  n2.asInstanceOf[Int])  +
+        |    f2(  n3.asInstanceOf[Int])  +
+        |    f2(               n4[Int])  + // "null"
+        |"-"                             +
+        |    f2(null.asInstanceOf[VCI])  +
+        |npe(f2(  n1.asInstanceOf[VCI])) + // SI-8097
+        |    f2(  n2.asInstanceOf[VCI])  +
+        |npe(f2(  n3.asInstanceOf[VCI])) + // SI-8097
+        |    f2(               n4[VCI])  + // "null"
+        |"-"                             +
+        |    f2(null.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f2(  n1.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f2(  n2.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f2(  n3.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f2(               n4[Unit]) + // "null"
+        |"-"                             +
+        |    f3(null.asInstanceOf[Int])  +
+        |    f3(  n1.asInstanceOf[Int])  +
+        |    f3(  n2.asInstanceOf[Int])  +
+        |    f3(  n3.asInstanceOf[Int])  +
+        |    f3(               n4[Int])  + // "null"
+        |"-"                             +
+        |    f3(null.asInstanceOf[VCI])  +
+        |npe(f3(  n1.asInstanceOf[VCI])) + // SI-8097
+        |    f3(  n2.asInstanceOf[VCI])  +
+        |npe(f3(  n3.asInstanceOf[VCI])) + // SI-8097
+        |    f3(               n4[VCI])  + // "null"
+        |"-"                             +
+        |    f3(null.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f3(  n1.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f3(  n2.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f3(  n3.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f3(               n4[Unit]) + // "null"
+        |"-"                             +
+        |    f4(null.asInstanceOf[Int])  +
+        |    f4(  n1.asInstanceOf[Int])  +
+        |    f4(  n2.asInstanceOf[Int])  +
+        |    f4(  n3.asInstanceOf[Int])  +
+        |    f4(               n4[Int])  +
+        |"-"                             +
+        |    f5(null.asInstanceOf[VCI])  +
+        |npe(f5(  n1.asInstanceOf[VCI])) + // SI-8097
+        |    f5(  n2.asInstanceOf[VCI])  +
+        |npe(f5(  n3.asInstanceOf[VCI])) + // SI-8097
+        |npe(f5(               n4[VCI])) + // SI-8097
+        |"-"                             +
+        |    f6(null.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f6(  n1.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f6(  n2.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f6(  n3.asInstanceOf[Unit]) + // "null", SI-9066
+        |    f6(               n4[Unit])   // "null"
+      """.stripMargin
+
+    assertEquals(run[String](code),
+      "0000null-0npe0npenull-nullnullnullnullnull-0000null-0npe0npenull-nullnullnullnullnull-0000null-0npe0npenull-nullnullnullnullnull-00000-0npe0npenpe-nullnullnullnullnull")
+  }
 }