SI-9702 Fix backend crash with classOf[T] annotation argument#5059
SI-9702 Fix backend crash with classOf[T] annotation argument#5059adriaanm merged 1 commit intoscala:2.12.xfrom
Conversation
| case StringTag => | ||
| assert(const.value != null, const) // TODO this invariant isn't documented in `case class Constant` | ||
| av.visit(name, const.stringValue) // `stringValue` special-cases null, but that execution path isn't exercised for a const with StringTag | ||
| case ClazzTag => av.visit(name, typeToBType(const.typeValue).toASMType) |
There was a problem hiding this comment.
Do we need the same treatment in genConstant, which contains the following?
case ClazzTag =>
val toPush: BType = {
typeToBType(const.typeValue) match {
case kind: PrimitiveBType => boxedClassOfPrimitive(kind)
case kind => kind
}
}
mnode.visitLdcInsn(toPush.toASMType)
There was a problem hiding this comment.
It turns out that code works fine. The reason is that erasure visits the Literal(Constant(..)) AST and replaces the type by a ClassNoArgsTypeRef to List. In the case of an annotation arg, the Constant(..) is part of a symbol's annotations and not touched by erasure.
|
Looks like we've also got a kindred bug in erasure: |
|
the value class type alias issue is easy to fix, will push another commit. |
|
review by @retronym. it includes a sketch for having run tests within junit, for now using toolboxes. maybe we should not rely on toolboxes, as java symbols are loaded through java reflection instead of the classfile parser - i fixed one bug in there. |
|
it's a bit of a rabbit hole, here's another one that crashes with current 2.12.x (only when probably won't finish that stuff today. |
|
as so often, once you start looking more details, there's plenty of little surprises :-) here's what happens for pre-req. erasure has two tree transformers:
now we're in the re-typing phase of erasure
that's how we end up with a non-erased (existential) type in the backend. if the |
66d5eff to
e4529ca
Compare
This commit fixes various issues with classOf literals and Java
annotations.
- Ensure that a Type within a ConstantType (i.e., a classOf literal)
is erased, so `classOf[List[Int]]` becomes `classOf[List]`.
- Ensure that no non-erased types are passed to `typeToBType` in the
backend. This happens for Java annotations: the annotation type and
`classOf` annotation arguments are not erased, the annotationInfos
of a symbol are not touched in the compiler pipeline.
- If T is an alias to a value class, ensure that `classOf[T]` erases
to the value class by calling `dealiasWiden` in erasure.
|
I think I went into all the glory details of this topic now, so ping @retronym. |
|
What a rabbit hole! This looks thoroughly thought through and tested. I'll review this in more detail tomorrow. |
|
LGTM @adriaanm Might be a contender for M4 |
|
Agreed -- merged! Now let's ship this puppy |
| tp | ||
| case ConstantType(ct) => | ||
| if (ct.tag == ClazzTag) ConstantType(Constant(apply(ct.typeValue))) | ||
| else tp |
There was a problem hiding this comment.
I think this broke Scala.js. In some cases, a classOf[Unit] becomes classOf[BoxedUnit] after erasure in 2.12.0-M4, but it did not up and until March 31, before this PR was merged (it stayed a classOf[Unit]). The "some" cases include when compiling java.lang.Void and scala.runtime.BoxedUnit in Scala.js; but does not include when compiling the hello world or the test suite. I did not try to minimize this, so far :-s
This causes the following 3 Scala.js tests to fail:
- org.scalajs.testsuite.javalib.lang.ClassTest
- org.scalajs.testsuite.scalalib.ArrayBuilderTest
- org.scalajs.testsuite.scalalib.ClassTagTest
See the full log here: https://scala-ci.typesafe.com/job/scala-2.12.x-integrate-community-build/364/consoleFull (grep for [scala-js] [error] Failed tests:).
At least for ClassTest, this happens because java.lang.Void.TYPE and scala.runtime.BoxedUnit.TYPE return the class scala.runtime.BoxedUnit instead of void. And that, because, when those classes were compiled, classOf[Unit] was transformed into classOf[BoxedUnit] during erasure. The source for these files are at https://github.com/scala-js/scala-js/blob/master/javalanglib/src/main/scala/java/lang/Void.scala and https://github.com/scala-js/scala-js/blob/master/library-aux/src/main/scala/scala/runtime/BoxedUnit.scala
Any idea?
There was a problem hiding this comment.
OK I can reproduce it. It happens precisely for final vals whose right-hand-side is a classOf[Unit]. Compiling
object Foo {
final val C = classOf[Unit]
}gives, after erasure:
object Foo extends Object {
def <init>(): helloworld.Foo.type = {
Foo.super.<init>();
()
};
final private[this] val C: Class(classOf[scala.runtime.BoxedUnit]) = classOf[scala.runtime.BoxedUnit];
final <stable> <accessor> def C(): Class(classOf[scala.runtime.BoxedUnit]) = classOf[scala.runtime.BoxedUnit]
};There was a problem hiding this comment.
I'm trying to reproduce:
$ cat Test.scala
object Test {
final val a = classOf[Unit]
final val b: Class[Unit] = classOf[Unit]
val c = classOf[Unit]
val d : Class[Unit] = classOf[Unit]
def main(args: Array[String]): Unit = {
val bu = classOf[runtime.BoxedUnit]
println(a)
println(bu)
println(a eq b)
println(a eq c)
println(a eq d)
println(a eq bu)
}
}
$ scala -version
Scala code runner version 2.12.0-M4 -- Copyright 2002-2016, LAMP/EPFL
$ scalac Test.scala
$ scala Test
void
class scala.runtime.BoxedUnit
true
true
true
false
There was a problem hiding this comment.
i do see the difference in the AST, i wonder if that's expected and what happens in the backend so that the two fields hold the same value:
$ cat Test.scala
object Test {
final val a = classOf[Unit]
final val b: Class[Unit] = classOf[Unit]
}
$ scalac Test.scala -Xprint:erasure
[[syntax trees at end of erasure]] // Test.scala
package <empty> {
object Test extends Object {
def <init>(): Test.type = {
Test.super.<init>();
()
};
final private[this] val a: Class(classOf[scala.runtime.BoxedUnit]) = classOf[scala.runtime.BoxedUnit];
final <stable> <accessor> def a(): Class(classOf[scala.runtime.BoxedUnit]) = classOf[scala.runtime.BoxedUnit];
final private[this] val b: Class = classOf[scala.Unit];
final <stable> <accessor> def b(): Class = Test.this.b
}
}
There was a problem hiding this comment.
it seems to be specific to Unit, doesn't happen for Int.
There was a problem hiding this comment.
Yes, in Scala/JVM you can only see it in the AST. Because of constant-folding for final vals coming from Scala-defined classes, the final val is not actually accessed at run-time. You can fully reproduce the bug by implementing an inherited def with the final val:
Welcome to Scala 2.12.0-M4 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_60).
Type in expressions for evaluation. Or try :help.
scala> trait Foo { def C: Class[_] }
defined trait Foo
scala> object Bar extends Foo { final val C = classOf[Unit] }
defined object Bar
scala> Bar.C // constant-folded, works
res0: Class[Unit] = void
scala> val foo: Foo = Bar
foo: Foo = Bar$@16cb0050
scala> foo.C // not constant-folded, does not work
res1: Class[_] = class scala.runtime.BoxedUnit
A
classOf[List[Int]]annotation argument is translated to a constantwith the
AliasArgsTypeRefas constant value.To obtain the corresponding ASM Type to emit as annotation argument, we
now go directly to the class symbol using
typeSymbol, instead ofcalling the generic
typeToBTypemethod, which expects onlynon-parameterized types.