diff options
author | Lukas Rytz <lukas.rytz@typesafe.com> | 2016-05-19 08:33:31 +0200 |
---|---|---|
committer | Lukas Rytz <lukas.rytz@typesafe.com> | 2016-05-19 08:33:31 +0200 |
commit | 960e4d76a382ff009c66346289db10b2ee598aa5 (patch) | |
tree | d15ddddd1bd1cea4fe6afc811a0771e9113eb24a | |
parent | 41c9a17e4f211fc24a931949a0819a0474cc004a (diff) | |
parent | a5fab1f588a6042ca924a78d225e85d0acddf5db (diff) | |
download | scala-960e4d76a382ff009c66346289db10b2ee598aa5.tar.gz scala-960e4d76a382ff009c66346289db10b2ee598aa5.tar.bz2 scala-960e4d76a382ff009c66346289db10b2ee598aa5.zip |
Merge pull request #5176 from lrytz/t9671
SI-9671, SI-7397 fix null.asInstanceOf[Int] when pt erases to Object
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/Erasure.scala | 10 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala | 21 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/TypeAdaptingTransformer.scala | 22 | ||||
-rw-r--r-- | test/junit/scala/BoxUnboxTest.scala | 30 | ||||
-rw-r--r-- | test/junit/scala/issues/BytecodeTest.scala | 10 | ||||
-rw-r--r-- | test/junit/scala/issues/RunTest.scala | 98 |
6 files changed, 155 insertions, 36 deletions
diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala index bc614dfc31..5e903946c1 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 { @@ -1143,6 +1141,10 @@ abstract class Erasure extends AddInterfaces else { val tree1 = preErase(tree) tree1 match { + case TypeApply(fun, targs @ List(targ)) if fun.symbol == Any_asInstanceOf && targ.tpe == UnitTpe => + // SI-9066 prevent transforming `o.asInstanceOf[Unit]` to `o.asInstanceOf[BoxedUnit]`. + // adaptMember will then replace the call by a reference to BoxedUnit.UNIT. + treeCopy.TypeApply(tree1, transform(fun), targs).clearType() case EmptyTree | TypeTree() => tree1 setType specialScalaErasure(tree1.tpe) case ArrayValue(elemtpt, trees) => 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..88b3037e69 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 @@ -106,14 +106,14 @@ class BoxUnboxTest { Unit.unbox({eff(); null}); chk() assertThrows[ClassCastException](Unit.unbox({eff(); ""})); chk() - val n1 = null.asInstanceOf[Unit] // SI-9066: should be UNIT, but currently null - assertThrows[AssertionError](assert(n1 == b)) // should not throw + val n1 = null.asInstanceOf[Unit] + assert(n1 == b) - val n2 = null.asInstanceOf[Unit] == b // SI-9066: should be true, but currently false - assertThrows[AssertionError](assert(n2)) // should not throw + val n2 = null.asInstanceOf[Unit] == b + assert(n2) def f(a: Any) = "" + a - val n3 = f(null.asInstanceOf[Unit]) // "null", should be "()". probably same cause as SI-602. - assertThrows[AssertionError](assertEquals(n3, "()")) // should not throw + val n3 = f(null.asInstanceOf[Unit]) + assertEquals(n3, "()") } } 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..3ebdc8a72f 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]) + + | f1( n1.asInstanceOf[Unit]) + + | f1( n2.asInstanceOf[Unit]) + + | f1( n3.asInstanceOf[Unit]) + + | 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]) + + | f2( n1.asInstanceOf[Unit]) + + | f2( n2.asInstanceOf[Unit]) + + | f2( n3.asInstanceOf[Unit]) + + | 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]) + + | f3( n1.asInstanceOf[Unit]) + + | f3( n2.asInstanceOf[Unit]) + + | f3( n3.asInstanceOf[Unit]) + + | 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]) + + | f6( n1.asInstanceOf[Unit]) + + | f6( n2.asInstanceOf[Unit]) + + | f6( n3.asInstanceOf[Unit]) + + | f6( n4[Unit]) // "null" + """.stripMargin + + assertEquals(run[String](code), + "0000null-0npe0npenull-()()()()null-0000null-0npe0npenull-()()()()null-0000null-0npe0npenull-()()()()null-00000-0npe0npenpe-()()()()null") + } } |