fix VM uint conversion size bug, stricter int gen on JS (#22150)

* fix VM uint conversion bug, stricter int gen on JS

fixes #19929

* fix float -> uint64 conversion too

* no need to mask to source type

* simpler diff with explanation, add test for described issue

compiler/jsgen.nim

@@ -2513,10 +2513,12 @@ proc genConv(p: PProc, n: PNode, r: var TCompRes) =
     elif src.kind == tyUInt64:
       r.res = "BigInt.asIntN(64, $1)" % [r.res]
   elif dest.kind == tyUInt64 and optJsBigInt64 in p.config.globalOptions:
-    if fromInt or fromUint:
+    if fromUint or src.kind in {tyBool, tyChar, tyEnum}:
       r.res = "BigInt($1)" % [r.res]
+    elif fromInt: # could be negative
+      r.res = "BigInt.asUintN(64, BigInt($1))" % [r.res]
     elif src.kind in {tyFloat..tyFloat64}:
-      r.res = "BigInt(Math.trunc($1))" % [r.res]
+      r.res = "BigInt.asUintN(64, BigInt(Math.trunc($1)))" % [r.res]
     elif src.kind == tyInt64:
       r.res = "BigInt.asUintN(64, $1)" % [r.res]
   elif toUint or dest.kind in tyFloat..tyFloat64:
@@ -2755,10 +2757,12 @@ proc genCast(p: PProc, n: PNode, r: var TCompRes) =
     elif src.kind == tyUInt64:
       r.res = "BigInt.asIntN(64, $1)" % [r.res]
   elif dest.kind == tyUInt64 and optJsBigInt64 in p.config.globalOptions:
-    if fromInt or fromUint:
+    if fromUint or src.kind in {tyBool, tyChar, tyEnum}:
       r.res = "BigInt($1)" % [r.res]
+    elif fromInt: # could be negative
+      r.res = "BigInt.asUintN(64, BigInt($1))" % [r.res]
     elif src.kind in {tyFloat..tyFloat64}:
-      r.res = "BigInt(Math.trunc($1))" % [r.res]
+      r.res = "BigInt.asUintN(64, BigInt(Math.trunc($1)))" % [r.res]
     elif src.kind == tyInt64:
       r.res = "BigInt.asUintN(64, $1)" % [r.res]
   elif dest.kind in tyFloat..tyFloat64:
@@ -2790,11 +2794,17 @@ proc gen(p: PProc, n: PNode, r: var TCompRes) =
       if optJsBigInt64 in p.config.globalOptions:
         r.res.add('n')
     of tyInt64:
-      r.res = rope(n.intVal)
+      let wrap = n.intVal < 0 # wrap negative integers with parens
+      if wrap: r.res.add '('
+      r.res.addInt n.intVal
       if optJsBigInt64 in p.config.globalOptions:
         r.res.add('n')
+      if wrap: r.res.add ')'
     else:
-      r.res = rope(n.intVal)
+      let wrap = n.intVal < 0 # wrap negative integers with parens
+      if wrap: r.res.add '('
+      r.res.addInt n.intVal
+      if wrap: r.res.add ')'
     r.kind = resExpr
   of nkNilLit:
     if isEmptyType(n.typ):

compiler/vm.nim

@@ -443,12 +443,16 @@ proc opConv(c: PCtx; dest: var TFullReg, src: TFullReg, desttyp, srctyp: PType):
       of tyFloat..tyFloat64:
         dest.intVal = int(src.floatVal)
       else:
-        let srcSize = getSize(c.config, styp)
         let destSize = getSize(c.config, desttyp)
-        let srcDist = (sizeof(src.intVal) - srcSize) * 8
         let destDist = (sizeof(dest.intVal) - destSize) * 8
         var value = cast[BiggestUInt](src.intVal)
-        value = (value shl srcDist) shr srcDist
+        when false:
+          # this would make uint64(-5'i8) evaluate to 251
+          # but at runtime, uint64(-5'i8) is 18446744073709551611
+          # so don't do it
+          let srcSize = getSize(c.config, styp)
+          let srcDist = (sizeof(src.intVal) - srcSize) * 8
+          value = (value shl srcDist) shr srcDist
         value = (value shl destDist) shr destDist
         dest.intVal = cast[BiggestInt](value)
     of tyBool:

tests/js/tneginthash.nim

@@ -0,0 +1,21 @@
+# issue #19929
+
+import std/[tables, hashes]
+
+type Foo = object
+  a: int
+
+proc hash(f: Foo): Hash =
+  var h: Hash = 0
+  h = h !& hash(f.a)
+  result = !$h
+
+proc transpose[T, S](data: array[T, S]): Table[S, T] =
+  for i, x in data:
+    result[x] = i
+
+const xs = [Foo(a: 5), Foo(a: -5)]
+const x = transpose(xs)
+
+doAssert x[Foo(a: -5)] == 1
+doAssert x[Foo(a: 5)] == 0

tests/stdlib/thashes.nim

@@ -29,6 +29,9 @@ block hashes:
     const wy123 = hashWangYi1(123)
     doAssert wy123 != 0
     doAssert hashWangYi1(123) == wy123
+    const wyNeg123 = hashWangYi1(-123)
+    doAssert wyNeg123 != 0
+    doAssert hashWangYi1(-123) == wyNeg123
 
 
   # "hashIdentity value incorrect at 456"

tests/vm/ttouintconv.nim

@@ -75,3 +75,10 @@ macro foo2() =
 
 foo()
 foo2()
+
+block:
+  const neg5VM = block:
+    let x = -5'i8
+    uint64(x)
+  let y = -5'i8
+  doAssert uint64(y) == neg5VM