Skip to content

Commit 508a70b

Browse files
committed
Restore the specified behavior of j.l.Class.isAssignableFrom.
The implemented behavior used to match the specification, but was changed in 9108b7c to fix scala-js#879. It turns out that the real culprit for scala-js#879 was not `isAssignableFrom`, but the implementation of `ClassTag.unapply`. Now that the latter has a Scala.js-friendly implementation in all versions of Scala for which it is not also broken on the JVM, we do not need `isAssignableFrom` to work around it. This makes sure that the behavior of `java.lang.Class` is not dependent on Scala as a source language. One partest does not pass out-of-the-box in 2.11.x if we do that: `run/t6318_primitives.scala`, but it works in 2.12.x. That happens because 2.12.x removed the overloads taking primitives, so at a source call site of `ct.unapply(5)`, 2.11.x would call `unapply(Int)` but 2.12+ would call `unapply(Any)`. It turns out that the `unapply`s taking primitives are not Scala.js-friendly enough for this partest (which has an overridden check file, btw). We fix this by "backporting" from 2.12 the removal of the `unapply`s taking any. In practice we cannot do that because we need to preserve binary compatibility, but we can implement it anyway by overriding them to delegate to the `unapply` taking `Any`.
1 parent 5d285d5 commit 508a70b

File tree

4 files changed

+55
-44
lines changed

4 files changed

+55
-44
lines changed

javalanglib/src/main/scala/java/lang/Class.scala

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,27 +30,8 @@ final class Class[A] private (data: ScalaJSClassData[A]) extends Object {
3030
data.isInstance(obj)
3131

3232
def isAssignableFrom(that: Class[_]): scala.Boolean =
33-
if (this.isPrimitive || that.isPrimitive) {
34-
/* This differs from the JVM specification to mimic the behavior of
35-
* runtime type tests of primitive numeric types.
36-
*/
37-
(this eq that) || {
38-
if (this eq classOf[scala.Short])
39-
(that eq classOf[scala.Byte])
40-
else if (this eq classOf[scala.Int])
41-
(that eq classOf[scala.Byte]) || (that eq classOf[scala.Short])
42-
else if (this eq classOf[scala.Float])
43-
(that eq classOf[scala.Byte]) || (that eq classOf[scala.Short]) ||
44-
(that eq classOf[scala.Int])
45-
else if (this eq classOf[scala.Double])
46-
(that eq classOf[scala.Byte]) || (that eq classOf[scala.Short]) ||
47-
(that eq classOf[scala.Int]) || (that eq classOf[scala.Float])
48-
else
49-
false
50-
}
51-
} else {
52-
this.isInstance(that.getFakeInstance())
53-
}
33+
if (this.isPrimitive || that.isPrimitive) this eq that
34+
else this.isInstance(that.getFakeInstance())
5435

5536
private def getFakeInstance(): Object =
5637
data.getFakeInstance()

scalalib/overrides-2.11/scala/reflect/ClassTag.scala

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -84,19 +84,15 @@ trait ClassTag[T] extends ClassManifestDeprecatedApis[T] with Equals with Serial
8484
else None
8585

8686
// TODO: deprecate overloads in 2.12.0, remove in 2.13.0
87-
def unapply(x: Byte) : Option[T] = unapplyImpl(x, classOf[Byte])
88-
def unapply(x: Short) : Option[T] = unapplyImpl(x, classOf[Short])
89-
def unapply(x: Char) : Option[T] = unapplyImpl(x, classOf[Char])
90-
def unapply(x: Int) : Option[T] = unapplyImpl(x, classOf[Int])
91-
def unapply(x: Long) : Option[T] = unapplyImpl(x, classOf[Long])
92-
def unapply(x: Float) : Option[T] = unapplyImpl(x, classOf[Float])
93-
def unapply(x: Double) : Option[T] = unapplyImpl(x, classOf[Double])
94-
def unapply(x: Boolean) : Option[T] = unapplyImpl(x, classOf[Boolean])
95-
def unapply(x: Unit) : Option[T] = unapplyImpl(x, classOf[Unit])
96-
97-
private[this] def unapplyImpl(x: Any, primitiveCls: java.lang.Class[_]): Option[T] =
98-
if (runtimeClass.isInstance(x) || runtimeClass.isAssignableFrom(primitiveCls)) Some(x.asInstanceOf[T])
99-
else None
87+
def unapply(x: Byte) : Option[T] = unapply(x: Any)
88+
def unapply(x: Short) : Option[T] = unapply(x: Any)
89+
def unapply(x: Char) : Option[T] = unapply(x: Any)
90+
def unapply(x: Int) : Option[T] = unapply(x: Any)
91+
def unapply(x: Long) : Option[T] = unapply(x: Any)
92+
def unapply(x: Float) : Option[T] = unapply(x: Any)
93+
def unapply(x: Double) : Option[T] = unapply(x: Any)
94+
def unapply(x: Boolean) : Option[T] = unapply(x: Any)
95+
def unapply(x: Unit) : Option[T] = unapply(x: Any)
10096

10197
// case class accessories
10298
override def canEqual(x: Any) = x.isInstanceOf[ClassTag[_]]

test-suite/js/src/test/scala/org/scalajs/testsuite/compiler/ReflectionTest.scala

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -125,16 +125,6 @@ class ReflectionTest {
125125
assertEquals(classOf[scala.runtime.BoxedUnit], ((): Any).getClass)
126126
}
127127

128-
@Test def class_isAssignableFrom_should_mimic_runtime_type_tests_behavior_issue_879(): Unit = {
129-
assertTrue(classOf[Short].isAssignableFrom(classOf[Byte]))
130-
assertTrue(classOf[Byte].isAssignableFrom(classOf[Byte]))
131-
assertFalse(classOf[Byte].isAssignableFrom(classOf[Short]))
132-
assertTrue(classOf[Int].isAssignableFrom(classOf[Byte]))
133-
assertTrue(classOf[Double].isAssignableFrom(classOf[Int]))
134-
assertFalse(classOf[Int].isAssignableFrom(classOf[Double]))
135-
assertFalse(classOf[Long].isAssignableFrom(classOf[Int]))
136-
}
137-
138128
@Test def getSuperclass_issue_1489(): Unit = {
139129
assertEquals(classOf[SomeParentClass], classOf[SomeChildClass].getSuperclass)
140130
assertNull(classOf[AnyRef].getSuperclass)

test-suite/shared/src/test/scala/org/scalajs/testsuite/javalib/lang/ClassTest.scala

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,18 @@ import scala.runtime.BoxedUnit
1414

1515
class ClassTest {
1616

17+
private val PrimitiveClassOfs = Seq(
18+
classOf[Unit],
19+
classOf[Boolean],
20+
classOf[Char],
21+
classOf[Byte],
22+
classOf[Short],
23+
classOf[Int],
24+
classOf[Long],
25+
classOf[Float],
26+
classOf[Double]
27+
)
28+
1729
@Test def getPrimitiveTypeName(): Unit = {
1830
assertEquals("void", classOf[Unit].getName)
1931
assertEquals("boolean", classOf[Boolean].getName)
@@ -57,6 +69,38 @@ class ClassTest {
5769
assertEquals("InnerClass", classOf[ClassTestClass#InnerClass].getSimpleName())
5870
}
5971

72+
@Test def isAssignableFrom(): Unit = {
73+
val SelectedClassOfs =
74+
PrimitiveClassOfs ++ Seq(classOf[Object], classOf[String])
75+
76+
// All Classes are assignable from themselves
77+
for (cls <- SelectedClassOfs) {
78+
assertTrue(s"$cls should be assignable from itself",
79+
cls.isAssignableFrom(cls))
80+
}
81+
82+
// Otherwise, if one side is a primitive, the result must be false
83+
for {
84+
left <- SelectedClassOfs
85+
right <- SelectedClassOfs
86+
if (left ne right) && (left.isPrimitive || right.isPrimitive)
87+
} {
88+
assertFalse(
89+
s"$left.isAssignableFrom($right) should be false",
90+
left.isAssignableFrom(right))
91+
}
92+
93+
assertTrue(classOf[Object].isAssignableFrom(classOf[String]))
94+
assertTrue(classOf[Seq[_]].isAssignableFrom(classOf[List[_]]))
95+
assertTrue(classOf[Object].isAssignableFrom(classOf[Array[String]]))
96+
assertTrue(classOf[Array[Seq[_]]].isAssignableFrom(classOf[Array[List[_]]]))
97+
98+
assertFalse(classOf[String].isAssignableFrom(classOf[Object]))
99+
assertFalse(classOf[List[_]].isAssignableFrom(classOf[Seq[_]]))
100+
assertFalse(classOf[Array[String]].isAssignableFrom(classOf[Object]))
101+
assertFalse(classOf[Array[List[_]]].isAssignableFrom(classOf[Array[Seq[_]]]))
102+
}
103+
60104
@Test def getComponentType(): Unit = {
61105
@noinline
62106
def testNoInline(clazz: Class[_], componentType: Class[_]): Unit =

0 commit comments

Comments
 (0)