Merge pull request #91 from sjrd/fix-hash-codes

Implement `IdentityHashCode` and fix the hashCode of primitives.

Author: sjrd

Diff for: build.sbt

@@ -247,15 +247,6 @@ lazy val IgnoredTestNames: Set[String] = {
     "org.scalajs.testsuite.javalib.lang.ObjectTest",
     // eqEqJLFloat/eqEqJLDouble failed: java.lang.AssertionError: null
     "org.scalajs.testsuite.compiler.RegressionTest",
-    // hashCode of floats and doubles
-    "org.scalajs.testsuite.javalib.lang.DoubleTest",
-    "org.scalajs.testsuite.javalib.lang.FloatTest",
-    "org.scalajs.testsuite.javalib.util.ArraysTest",
-    "org.scalajs.testsuite.typedarray.ArraysTest",
-    // hashCode of bigints and symbols
-    "org.scalajs.testsuite.javalib.lang.ObjectJSTest",
-    // Various issues with identityHashCode
-    "org.scalajs.testsuite.javalib.lang.SystemTest",
     // TypeError: WebAssembly objects are opaque
     "org.scalajs.testsuite.javalib.lang.SystemJSTest",
     // throwablesAreTrueErrors failed: org.junit.ComparisonFailure: expected:<[object [Error]]> but was:<[object [Object]]>

Diff for: test-suite/src/main/scala/testsuite/core/HijackedClassesDispatchTest.scala

@@ -21,7 +21,8 @@ object HijackedClassesDispatchTest {
         testHashCode(54321, 54321) &&
         testHashCode("foo", 101574) &&
         testHashCode(obj, 123) &&
-        testHashCode(obj2, 42) &&
+        testHashCode(obj2, 1) && // first object for which we ask idHashCode
+        testHashCode(obj2, 1) && // it is also 1 the second time we ask
         testHashCode('A', 65) &&
         testIntValue(Int.box(5), 5) &&
         testIntValue(Long.box(6L), 6) &&

Diff for: wasm/src/main/scala/WebAssemblyLinkerBackend.scala

@@ -45,7 +45,11 @@ final class WebAssemblyLinkerBackend(
     factory.multiple(
       factory.instantiateClass(ClassClass, ClassCtor),
       factory.instantiateClass(CharBoxClass, CharBoxCtor),
-      factory.instantiateClass(LongBoxClass, LongBoxCtor)
+      factory.instantiateClass(LongBoxClass, LongBoxCtor),
+
+      // See genIdentityHashCode in HelperFunctions
+      factory.callMethodStatically(BoxedDoubleClass, hashCodeMethodName),
+      factory.callMethodStatically(BoxedStringClass, hashCodeMethodName)
     )
   }
 

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

@@ -21,7 +21,9 @@ object EmbeddedConstants {
   final val JSValueTypeString = 2
   final val JSValueTypeNumber = 3
   final val JSValueTypeUndefined = 4
-  final val JSValueTypeOther = 5
+  final val JSValueTypeBigInt = 5
+  final val JSValueTypeSymbol = 6
+  final val JSValueTypeOther = 7
 
   // Values for `typeData.kind`
 

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

@@ -33,6 +33,7 @@ object HelperFunctions {
     genNewArrayOfThisClass()
     genAnyGetClass()
     genNewArrayObject()
+    genIdentityHashCode()
   }
 
   private def genStringLiteral()(implicit ctx: WasmContext): Unit = {
@@ -1383,6 +1384,137 @@ object HelperFunctions {
     fctx.buildAndAddToContext()
   }
 
+  /** `identityHashCode`: `anyref -> i32`.
+    *
+    * This is the implementation of `IdentityHashCode`. It is also used to compute the `hashCode()`
+    * of primitive values when dispatch is required (i.e., when the receiver type is not known to be
+    * a specific primitive or hijacked class), so it must be consistent with the implementations of
+    * `hashCode()` in hijacked classes.
+    *
+    * For `String` and `Double`, we actually call the hijacked class methods, as they are a bit
+    * involved. For `Boolean` and `Void`, we hard-code a copy here.
+    */
+  def genIdentityHashCode()(implicit ctx: WasmContext): Unit = {
+    import IRTrees.MemberNamespace.Public
+    import SpecialNames.hashCodeMethodName
+    import WasmTypeName._
+    import WasmFieldIdx.typeData._
+
+    // A global exclusively used by this function
+    ctx.addGlobal(
+      WasmGlobal(
+        WasmGlobalName.lastIDHashCode,
+        WasmInt32,
+        WasmExpr(List(I32_CONST(0))),
+        isMutable = true
+      )
+    )
+
+    val fctx = WasmFunctionContext(
+      WasmFunctionName.identityHashCode,
+      List("obj" -> WasmRefType.anyref),
+      List(WasmInt32)
+    )
+
+    val List(objParam) = fctx.paramIndices
+
+    import fctx.instrs
+
+    val objNonNullLocal = fctx.addLocal("objNonNull", WasmRefType.any)
+    val resultLocal = fctx.addLocal("result", WasmInt32)
+
+    // If `obj` is `null`, return 0 (by spec)
+    fctx.block(WasmRefType.any) { nonNullLabel =>
+      instrs += LOCAL_GET(objParam)
+      instrs += BR_ON_NON_NULL(nonNullLabel)
+      instrs += I32_CONST(0)
+      instrs += RETURN
+    }
+    instrs += LOCAL_TEE(objNonNullLocal)
+
+    // If `obj` is one of our objects, skip all the jsValueType tests
+    instrs += REF_TEST(WasmRefType(WasmHeapType.ObjectType))
+    instrs += I32_EQZ
+    fctx.ifThen() {
+      fctx.switch() { () =>
+        instrs += LOCAL_GET(objNonNullLocal)
+        instrs += CALL(WasmFunctionName.jsValueType)
+      }(
+        List(JSValueTypeFalse) -> { () =>
+          instrs += I32_CONST(1237) // specified by jl.Boolean.hashCode()
+          instrs += RETURN
+        },
+        List(JSValueTypeTrue) -> { () =>
+          instrs += I32_CONST(1231) // specified by jl.Boolean.hashCode()
+          instrs += RETURN
+        },
+        List(JSValueTypeString) -> { () =>
+          instrs += LOCAL_GET(objNonNullLocal)
+          instrs += CALL(WasmFunctionName(Public, IRNames.BoxedStringClass, hashCodeMethodName))
+          instrs += RETURN
+        },
+        List(JSValueTypeNumber) -> { () =>
+          instrs += LOCAL_GET(objNonNullLocal)
+          instrs += CALL(WasmFunctionName.unbox(IRTypes.DoubleRef))
+          instrs += CALL(WasmFunctionName(Public, IRNames.BoxedDoubleClass, hashCodeMethodName))
+          instrs += RETURN
+        },
+        List(JSValueTypeUndefined) -> { () =>
+          instrs += I32_CONST(0) // specified by jl.Void.hashCode(), Scala.js only
+          instrs += RETURN
+        },
+        List(JSValueTypeBigInt) -> { () =>
+          instrs += LOCAL_GET(objNonNullLocal)
+          instrs += CALL(WasmFunctionName.bigintHashCode)
+          instrs += RETURN
+        },
+        List(JSValueTypeSymbol) -> { () =>
+          fctx.block() { descriptionIsNullLabel =>
+            instrs += LOCAL_GET(objNonNullLocal)
+            instrs += CALL(WasmFunctionName.symbolDescription)
+            instrs += BR_ON_NULL(descriptionIsNullLabel)
+            instrs += CALL(WasmFunctionName(Public, IRNames.BoxedStringClass, hashCodeMethodName))
+            instrs += RETURN
+          }
+          instrs += I32_CONST(0)
+          instrs += RETURN
+        }
+      ) { () =>
+        // JSValueTypeOther -- fall through to using idHashCodeMap
+        ()
+      }
+    }
+
+    // If we get here, use the idHashCodeMap
+
+    // Read the existing idHashCode, if one exists
+    instrs += GLOBAL_GET(WasmGlobalName.idHashCodeMap)
+    instrs += LOCAL_GET(objNonNullLocal)
+    instrs += CALL(WasmFunctionName.idHashCodeGet)
+    instrs += LOCAL_TEE(resultLocal)
+
+    // If it is 0, there was no recorded idHashCode yet; allocate a new one
+    instrs += I32_EQZ
+    fctx.ifThen() {
+      // Allocate a new idHashCode
+      instrs += GLOBAL_GET(WasmGlobalName.lastIDHashCode)
+      instrs += I32_CONST(1)
+      instrs += I32_ADD
+      instrs += LOCAL_TEE(resultLocal)
+      instrs += GLOBAL_SET(WasmGlobalName.lastIDHashCode)
+
+      // Store it for next time
+      instrs += GLOBAL_GET(WasmGlobalName.idHashCodeMap)
+      instrs += LOCAL_GET(objNonNullLocal)
+      instrs += LOCAL_GET(resultLocal)
+      instrs += CALL(WasmFunctionName.idHashCodeSet)
+    }
+
+    instrs += LOCAL_GET(resultLocal)
+
+    fctx.buildAndAddToContext()
+  }
+
   /** Generate type inclusion test for interfaces.
     *
     * The expression `isInstanceOf[<interface>]` will be compiled to a CALL to the function

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

@@ -11,15 +11,15 @@ object LoaderContent {
 
   private def stringContent: String = {
     raw"""
-// Specified by java.lang.String.hashCode()
-function stringHashCode(s) {
+// This implementation follows no particular specification, but is the same as the JS backend.
+// It happens to coincide with java.lang.Long.hashCode() for common values.
+function bigintHashCode(x) {
   var res = 0;
-  var mul = 1;
-  var i = (s.length - 1) | 0;
-  while ((i >= 0)) {
-    res = ((res + Math.imul(s.charCodeAt(i), mul)) | 0);
-    mul = Math.imul(31, mul);
-    i = (i - 1) | 0;
+  if (x < 0n)
+    x = ~x;
+  while (x !== 0n) {
+    res ^= Number(BigInt.asIntN(32, x));
+    x >>= 32n;
   }
   return res;
 }
@@ -136,22 +136,21 @@ const scalaJSHelpers = {
       return x | 0; // JSValueTypeFalse or JSValueTypeTrue
     if (typeof x === 'undefined')
       return $JSValueTypeUndefined;
+    if (typeof x === 'bigint')
+      return $JSValueTypeBigInt;
+    if (typeof x === 'symbol')
+      return $JSValueTypeSymbol;
     return $JSValueTypeOther;
   },
 
-  // Hash code, because it is overridden in all hijacked classes
-  // Specified by the hashCode() method of the corresponding hijacked classes
-  jsValueHashCode: (x) => {
-    if (typeof x === 'number')
-      return x | 0; // TODO make this compliant for floats
-    if (typeof x === 'string')
-      return stringHashCode(x);
-    if (typeof x === 'boolean')
-      return x ? 1231 : 1237;
-    if (typeof x === 'undefined')
-      return 0;
-    return 42; // for any JS object
+  // Identity hash code
+  bigintHashCode: bigintHashCode,
+  symbolDescription: (x) => {
+    var desc = x.description;
+    return (desc === void 0) ? null : desc;
   },
+  idHashCodeGet: (map, obj) => map.get(obj) | 0, // undefined becomes 0
+  idHashCodeSet: (map, obj, value) => map.set(obj, value),
 
   // JS interop
   jsGlobalRefGet: (globalRefName) => (new Function("return " + globalRefName))(),
@@ -280,8 +279,9 @@ const scalaJSHelpers = {
 }
 
 export async function load(wasmFileURL, importedModules) {
+  const myScalaJSHelpers = { ...scalaJSHelpers, idHashCodeMap: new WeakMap() };
   const importsObj = {
-    "__scalaJSHelpers": scalaJSHelpers,
+    "__scalaJSHelpers": myScalaJSHelpers,
     "__scalaJSImports": importedModules,
   };
   const resolvedURL = new URL(wasmFileURL, import.meta.url);

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

@@ -21,4 +21,6 @@ object SpecialNames {
   val JSExceptionClass = ClassName("scala.scalajs.js.JavaScriptException")
   val JSExceptionCtor = MethodName.constructor(List(ClassRef(ObjectClass)))
   val JSExceptionField = FieldName(JSExceptionClass, SimpleFieldName("exception"))
+
+  val hashCodeMethodName = MethodName("hashCode", Nil, IntRef)
 }

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

@@ -40,7 +40,6 @@ object WasmExpressionBuilder {
   private val ObjectRef = IRTypes.ClassRef(IRNames.ObjectClass)
   private val BoxedStringRef = IRTypes.ClassRef(IRNames.BoxedStringClass)
   private val toStringMethodName = IRNames.MethodName("toString", Nil, BoxedStringRef)
-  private val hashCodeMethodName = IRNames.MethodName("hashCode", Nil, IRTypes.IntRef)
   private val equalsMethodName = IRNames.MethodName("equals", List(ObjectRef), IRTypes.BooleanRef)
   private val compareToMethodName = IRNames.MethodName("compareTo", List(ObjectRef), IRTypes.IntRef)
 
@@ -310,6 +309,12 @@ private class WasmExpressionBuilder private (
     }
   }
 
+  /** Generates the code an `Apply` call where the receiver's type is not statically known to be a
+    * primitive or hijacked class.
+    *
+    * In that case, there is always at least a vtable/itable-based dispatch. It may also contain
+    * primitive-based dispatch if the receiver's type is an ancestor of a hijacked class.
+    */
   private def genApplyNonPrim(t: IRTrees.Apply): IRTypes.Type = {
     implicit val pos: Position = t.pos
 
@@ -378,7 +383,11 @@ private class WasmExpressionBuilder private (
       genArgs(t.args, t.method.name)
       genTableDispatch(receiverClassInfo, t.method.name, receiverLocalForDispatch)
     } else {
-      /* Hijacked class dispatch codegen
+      /* Here the receiver's type is an ancestor of a hijacked class (or `any`,
+       * which is treated as `jl.Object`).
+       *
+       * We must emit additional dispatch for the possible primitive values.
+       *
        * The overall structure of the generated code is as follows:
        *
        * block resultType $done
@@ -443,7 +452,17 @@ private class WasmExpressionBuilder private (
           argsLocals
         } // end block labelNotOurObject
 
-        // Now we have a value that is not one of our objects; the (ref any) is still on the stack
+        /* Now we have a value that is not one of our objects, so it must be
+         * a JavaScript value whose representative class extends/implements the
+         * receiver class. It may be a primitive instance of a hijacked class, or
+         * any other value (whose representative class is therefore `jl.Object`).
+         *
+         * It is also *not* `char` or `long`, since those would reach
+         * `genApplyNonPrim` in their boxed form, and therefore they are
+         * "ourObject".
+         *
+         * The (ref any) is still on the stack.
+         */
 
         if (t.method.name == toStringMethodName) {
           // By spec, toString() is special
@@ -499,15 +518,16 @@ private class WasmExpressionBuilder private (
           }
         } else {
           /* It must be a method of j.l.Object and it can be any value.
-           * hashCode() and equals() are overridden in all hijacked classes; we
-           * use dedicated JavaScript helpers for those.
+           * hashCode() and equals() are overridden in all hijacked classes.
+           * We use `identityHashCode` for `hashCode` and `Object.is` for `equals`,
+           * as they coincide with the respective specifications (on purpose).
            * The other methods are never overridden and can be statically
            * resolved to j.l.Object.
            */
           pushArgs(argsLocals)
           t.method.name match {
-            case `hashCodeMethodName` =>
-              instrs += CALL(WasmFunctionName.jsValueHashCode)
+            case SpecialNames.hashCodeMethodName =>
+              instrs += CALL(WasmFunctionName.identityHashCode)
             case `equalsMethodName` =>
               instrs += CALL(WasmFunctionName.is)
             case _ =>
@@ -1759,13 +1779,9 @@ private class WasmExpressionBuilder private (
   }
 
   private def genIdentityHashCode(tree: IRTrees.IdentityHashCode): IRTypes.Type = {
-    /* TODO We should allocate ID hash codes and store them. We will probably
-     * have to store them as an additional field in all objects, together with
-     * the vtable and itable pointers.
-     */
+    // TODO Avoid dispatch when we know a more precise type than any
     genTree(tree.expr, IRTypes.AnyType)
-    instrs += DROP
-    instrs += I32_CONST(42)
+    instrs += CALL(WasmFunctionName.identityHashCode)
 
     IRTypes.IntType
   }

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

@@ -76,6 +76,12 @@ object Names {
 
     val arrayClassITable: WasmGlobalName =
       new WasmGlobalName("itable.A")
+
+    val lastIDHashCode: WasmGlobalName =
+      new WasmGlobalName("lastIDHashCode")
+
+    val idHashCodeMap: WasmGlobalName =
+      new WasmGlobalName("idHashCodeMap")
   }
 
   // final case class WasmGlobalName private (val name: String) extends WasmName(name) {
@@ -181,7 +187,10 @@ object Names {
     val isString = helper("isString")
 
     val jsValueType = helper("jsValueType")
-    val jsValueHashCode = helper("jsValueHashCode")
+    val bigintHashCode = helper("bigintHashCode")
+    val symbolDescription = helper("symbolDescription")
+    val idHashCodeGet = helper("idHashCodeGet")
+    val idHashCodeSet = helper("idHashCodeSet")
 
     val jsGlobalRefGet = helper("jsGlobalRefGet")
     val jsGlobalRefSet = helper("jsGlobalRefSet")
@@ -268,6 +277,7 @@ object Names {
     val newArrayOfThisClass = helper("newArrayOfThisClass")
     val anyGetClass = helper("anyGetClass")
     val newArrayObject = helper("newArrayObject")
+    val identityHashCode = helper("identityHashCode")
   }
 
   final case class WasmFieldName private (override private[wasm4s] val name: String)