Skip to content
This repository was archived by the owner on Jul 12, 2024. It is now read-only.

Commit 71f17cd

Browse files
committed
Fix static type tests for ancestors of hijacked classes.
This commit fixes the result of `x.isInstanceOf[C]` where `x` is a primitive and `C` is an ancestor of the hijacked class for `x`. For `jl.Number`, which is the only non-`Object` class that is an ancestor of a hijacked class, we add a special case in the codegen. For interfaces, we add generic code in their `genInstanceTest` function, based on their `specialInstanceTypes`. This means we have a second place where we need to compute `specialInstanceTypes`. We move it to the preprocessor, and also use it to generically compute `isAncestorOfHijackedClass`. This removes the hard-code set that we had before. Fix #74. Fix #84. Fix #92.
1 parent 782d6ca commit 71f17cd

File tree

6 files changed

+117
-56
lines changed

6 files changed

+117
-56
lines changed

Diff for: build.sbt

-6
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,6 @@ lazy val IgnoredTestNames: Set[String] = {
228228
Set(
229229
// reflective call: should be throw an exception when reflective proxy not found
230230
"org.scalajs.testsuite.compiler.WasPublicBeforeTyperTestScala2",
231-
// javaLangNumber failed: java.lang.AssertionError: 1, class java.lang.Number expected:<true> but was:<false>
232-
"org.scalajs.testsuite.compiler.RuntimeTypeTestsTest",
233231
// Various run-time errors and JS exceptions
234232
"org.scalajs.testsuite.compiler.InteroperabilityTest",
235233
"org.scalajs.testsuite.compiler.RegressionJSTest",
@@ -245,17 +243,13 @@ lazy val IgnoredTestNames: Set[String] = {
245243
// getClass for Box classes
246244
"org.scalajs.testsuite.javalib.lang.ClassTest",
247245
"org.scalajs.testsuite.javalib.lang.ObjectTest",
248-
// eqEqJLFloat/eqEqJLDouble failed: java.lang.AssertionError: null
249-
"org.scalajs.testsuite.compiler.RegressionTest",
250246
// TypeError: WebAssembly objects are opaque
251247
"org.scalajs.testsuite.javalib.lang.SystemJSTest",
252248
// throwablesAreTrueErrors failed: org.junit.ComparisonFailure: expected:<[object [Error]]> but was:<[object [Object]]>
253249
// throwablesAreJSErrors failed: java.lang.AssertionError: null
254250
"org.scalajs.testsuite.javalib.lang.ThrowableJSTest",
255251
// keepBreakToLabelWithinFinallyBlock_Issue2689 failed: java.lang.AssertionError: expected:<2> but was:<1>
256252
"org.scalajs.testsuite.compiler.OptimizerTest",
257-
// nonUnitBoxedPrimitiveValuesAreSerializable failed: java.lang.AssertionError: Boolean
258-
"org.scalajs.testsuite.javalib.io.SerializableTest",
259253
// No support for stack traces
260254
"org.scalajs.testsuite.library.StackTraceTest",
261255
)

Diff for: wasm/src/main/scala/ir2wasm/HelperFunctions.scala

+39-5
Original file line numberDiff line numberDiff line change
@@ -1614,6 +1614,8 @@ object HelperFunctions {
16141614
def genInstanceTest(clazz: LinkedClass)(implicit ctx: WasmContext): Unit = {
16151615
assert(clazz.kind == ClassKind.Interface)
16161616

1617+
val classInfo = ctx.getClassInfo(clazz.className)
1618+
16171619
val fctx = WasmFunctionContext(
16181620
Names.WasmFunctionName.instanceTest(clazz.name.name),
16191621
List("expr" -> WasmRefType.anyref),
@@ -1623,9 +1625,9 @@ object HelperFunctions {
16231625

16241626
import fctx.instrs
16251627

1626-
val itables = fctx.addSyntheticLocal(
1627-
Types.WasmRefType.nullable(WasmArrayType.itables.name)
1628-
)
1628+
val itables = fctx.addLocal("itables", WasmRefType.nullable(WasmArrayType.itables.name))
1629+
val exprNonNullLocal = fctx.addLocal("exprNonNull", WasmRefType.any)
1630+
16291631
val itableIdx = ctx.getItableIdx(clazz.name.name)
16301632
fctx.block(WasmRefType.anyref) { testFail =>
16311633
// if expr is not an instance of Object, return false
@@ -1655,8 +1657,40 @@ object HelperFunctions {
16551657
instrs += I32_CONST(1)
16561658
instrs += RETURN
16571659
} // test fail
1658-
instrs += DROP
1659-
instrs += I32_CONST(0) // false
1660+
1661+
if (classInfo.isAncestorOfHijackedClass) {
1662+
/* It could be a hijacked class instance that implements this interface.
1663+
* Test whether `jsValueType(expr)` is in the `specialInstanceTypes` bitset.
1664+
* In other words, return `(1 << jsValueType(expr)) != 0`.
1665+
*/
1666+
val anyRefToVoidSig =
1667+
WasmFunctionSignature(List(WasmRefType.anyref), Nil)
1668+
1669+
fctx.block(anyRefToVoidSig) { isNullLabel =>
1670+
// exprNonNull := expr; branch to isNullLabel if it is null
1671+
instrs += BR_ON_NULL(isNullLabel)
1672+
instrs += LOCAL_SET(exprNonNullLocal)
1673+
1674+
// Load 1 << jsValueType(expr)
1675+
instrs += I32_CONST(1)
1676+
instrs += LOCAL_GET(exprNonNullLocal)
1677+
instrs += CALL(WasmFunctionName.jsValueType)
1678+
instrs += I32_SHL
1679+
1680+
// return (... & specialInstanceTypes) != 0
1681+
instrs += I32_CONST(classInfo.specialInstanceTypes)
1682+
instrs += I32_AND
1683+
instrs += I32_CONST(0)
1684+
instrs += I32_NE
1685+
instrs += RETURN
1686+
}
1687+
1688+
instrs += I32_CONST(0) // false
1689+
} else {
1690+
instrs += DROP
1691+
instrs += I32_CONST(0) // false
1692+
}
1693+
16601694
fctx.buildAndAddToContext()
16611695
}
16621696

Diff for: wasm/src/main/scala/ir2wasm/Preprocessor.scala

+21
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import org.scalajs.ir.Traversers
1010

1111
import org.scalajs.linker.standard.LinkedClass
1212

13+
import EmbeddedConstants._
1314
import WasmContext._
1415

1516
object Preprocessor {
@@ -101,6 +102,26 @@ object Preprocessor {
101102
)
102103
)
103104

105+
// Update specialInstanceTypes for ancestors of hijacked classes
106+
if (clazz.kind == ClassKind.HijackedClass) {
107+
def addSpecialInstanceTypeOnAllAncestors(jsValueType: Int): Unit =
108+
clazz.ancestors.foreach(ctx.getClassInfo(_).addSpecialInstanceType(jsValueType))
109+
110+
clazz.className match {
111+
case IRNames.BoxedBooleanClass =>
112+
addSpecialInstanceTypeOnAllAncestors(JSValueTypeFalse)
113+
addSpecialInstanceTypeOnAllAncestors(JSValueTypeTrue)
114+
case IRNames.BoxedStringClass =>
115+
addSpecialInstanceTypeOnAllAncestors(JSValueTypeString)
116+
case IRNames.BoxedDoubleClass =>
117+
addSpecialInstanceTypeOnAllAncestors(JSValueTypeNumber)
118+
case IRNames.BoxedUnitClass =>
119+
addSpecialInstanceTypeOnAllAncestors(JSValueTypeUndefined)
120+
case _ =>
121+
()
122+
}
123+
}
124+
104125
/* Work around https://github.com/scala-js/scala-js/issues/4972
105126
* Manually mark all ancestors of instantiated classes as having instances.
106127
*/

Diff for: wasm/src/main/scala/ir2wasm/WasmBuilder.scala

+2-30
Original file line numberDiff line numberDiff line change
@@ -158,35 +158,7 @@ class WasmBuilder {
158158
val className = clazz.className
159159
val classInfo = ctx.getClassInfo(className)
160160

161-
/* See the `isInstance` helper. `specialInstanceTypes` is a bitset of the
162-
* `jsValueType`s corresponding to hijacked classes that extend this class.
163-
* For example, if this class is `Comparable`, we want the bitset to contain
164-
* the values for `boolean`, `string` and `number` (but not `undefined`),
165-
* because `jl.Boolean`, `jl.String` and `jl.Double` implement `Comparable`.
166-
*
167-
* When testing whether a `value` is a `Comparable`, `isInstance` will
168-
* compute the `jsValueType(value)` and test whether it is part of the bit
169-
* set, using `((1 << jsValueType(value)) & specialInstanceTypes) != 0`.
170-
*/
171-
val specialInstanceTypes = {
172-
if (!classInfo.isAncestorOfHijackedClass) {
173-
// fast path
174-
0
175-
} else {
176-
var bits = 0
177-
if (ctx.getClassInfo(IRNames.BoxedBooleanClass).ancestors.contains(className))
178-
bits |= ((1 << JSValueTypeFalse) | (1 << JSValueTypeTrue))
179-
if (ctx.getClassInfo(IRNames.BoxedStringClass).ancestors.contains(className))
180-
bits |= (1 << JSValueTypeString)
181-
if (ctx.getClassInfo(IRNames.BoxedDoubleClass).ancestors.contains(className))
182-
bits |= (1 << JSValueTypeNumber)
183-
if (ctx.getClassInfo(IRNames.BoxedUnitClass).ancestors.contains(className))
184-
bits |= (1 << JSValueTypeUndefined)
185-
bits
186-
}
187-
}
188-
189-
val kind = clazz.className match {
161+
val kind = className match {
190162
case IRNames.ObjectClass => KindObject
191163
case IRNames.BoxedUnitClass => KindBoxedUnit
192164
case IRNames.BoxedBooleanClass => KindBoxedBoolean
@@ -209,7 +181,7 @@ class WasmBuilder {
209181

210182
genTypeDataFieldValues(
211183
kind,
212-
specialInstanceTypes,
184+
classInfo.specialInstanceTypes,
213185
IRTypes.ClassRef(clazz.className),
214186
vtableElems
215187
)

Diff for: wasm/src/main/scala/ir2wasm/WasmExpressionBuilder.scala

+14
Original file line numberDiff line numberDiff line change
@@ -1300,6 +1300,20 @@ private class WasmExpressionBuilder private (
13001300
instrs += I32_CONST(1)
13011301
instrs += I32_XOR
13021302

1303+
case IRTypes.ClassType(JLNumberClass) =>
1304+
/* Special case: the only non-Object *class* that is an ancestor of a
1305+
* hijacked class. We need to accept `number` primitives here.
1306+
*/
1307+
val tempLocal = fctx.addSyntheticLocal(Types.WasmRefType.anyref)
1308+
instrs += LOCAL_TEE(tempLocal)
1309+
instrs += REF_TEST(Types.WasmRefType(WasmStructTypeName.forClass(JLNumberClass)))
1310+
fctx.ifThenElse(Types.WasmInt32) {
1311+
instrs += I32_CONST(1)
1312+
} {
1313+
instrs += LOCAL_GET(tempLocal)
1314+
instrs += CALL(WasmFunctionName.typeTest(IRTypes.DoubleRef))
1315+
}
1316+
13031317
case IRTypes.ClassType(testClassName) =>
13041318
IRTypes.BoxedClassToPrimType.get(testClassName) match {
13051319
case Some(primType) =>

Diff for: wasm/src/main/scala/wasm4s/WasmContext.scala

+41-15
Original file line numberDiff line numberDiff line change
@@ -702,20 +702,6 @@ class WasmContext(val module: WasmModule) extends TypeDefinableWasmContext {
702702
object WasmContext {
703703
private val classFieldOffset = 2 // vtable, itables
704704

705-
private val AncestorsOfHijackedClasses: Set[IRNames.ClassName] = {
706-
// We hard-code this for now, but ideally we should derive it
707-
IRNames.HijackedClasses ++
708-
Set(
709-
IRNames.ObjectClass,
710-
IRNames.SerializableClass,
711-
IRNames.ClassName("java.lang.CharSequence"),
712-
IRNames.ClassName("java.lang.Comparable"),
713-
IRNames.ClassName("java.lang.Number"),
714-
IRNames.ClassName("java.lang.constant.Constable"),
715-
IRNames.ClassName("java.lang.constant.ConstantDesc")
716-
)
717-
}
718-
719705
final class WasmClassInfo(
720706
val name: IRNames.ClassName,
721707
val kind: ClassKind,
@@ -741,7 +727,47 @@ object WasmContext {
741727

742728
def hasInstances: Boolean = _hasInstances
743729

744-
def isAncestorOfHijackedClass: Boolean = AncestorsOfHijackedClasses.contains(name)
730+
private var _specialInstanceTypes: Int = 0
731+
732+
def addSpecialInstanceType(jsValueType: Int): Unit =
733+
_specialInstanceTypes |= (1 << jsValueType)
734+
735+
/** A bitset of the `jsValueType`s corresponding to hijacked classes that extend this class.
736+
*
737+
* This value is used for instance tests against this class. A JS value `x` is an instance of
738+
* this type iff `jsValueType(x)` is a member of this bitset. Because of how a bitset works,
739+
* this means testing the following formula:
740+
*
741+
* {{{
742+
* ((1 << jsValueType(x)) & specialInstanceTypes) != 0
743+
* }}}
744+
*
745+
* For example, if this class is `Comparable`, we want the bitset to contain the values for
746+
* `boolean`, `string` and `number` (but not `undefined`), because `jl.Boolean`, `jl.String`
747+
* and `jl.Double` implement `Comparable`.
748+
*
749+
* This field is initialized with 0, and augmented during preprocessing by calls to
750+
* `addSpecialInstanceType`.
751+
*
752+
* This technique is used both for static `isInstanceOf` tests as well as reflective tests
753+
* through `Class.isInstance`. For the latter, this value is stored in
754+
* `typeData.specialInstanceTypes`. For the former, it is embedded as a constant in the
755+
* generated code.
756+
*
757+
* See the `isInstance` and `genInstanceTest` helpers.
758+
*
759+
* Special cases: this value remains 0 for all the numeric hijacked classes except `jl.Double`,
760+
* since `jsValueType(x) == JSValueTypeNumber` is not enough to deduce that
761+
* `x.isInstanceOf[Int]`, for example.
762+
*/
763+
def specialInstanceTypes: Int = _specialInstanceTypes
764+
765+
/** Is this class an ancestor of any hijacked class?
766+
*
767+
* This includes but is not limited to the hijacked classes themselves, as well as `jl.Object`.
768+
*/
769+
def isAncestorOfHijackedClass: Boolean =
770+
specialInstanceTypes != 0 || kind == ClassKind.HijackedClass
745771

746772
def isInterface = kind == ClassKind.Interface
747773

0 commit comments

Comments
 (0)