From c44d564761fc986f32538e5cc29da83c9ea10490 Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Fri, 16 Apr 2021 05:21:26 -0700
Subject: [PATCH] std/hashes: hash(ref|ptr|pointer) + other improvements
 (#17731)

---
 lib/pure/hashes.nim                 | 72 ++++++++++++++++++++---------
 testament/lib/stdtest/testutils.nim |  4 ++
 tests/collections/ttables.nim       | 14 ++++++
 tests/stdlib/thashes.nim            | 22 ++++++++-
 tests/stdlib/tstrutils.nim          |  6 +--
 5 files changed, 89 insertions(+), 29 deletions(-)

diff --git a/lib/pure/hashes.nim b/lib/pure/hashes.nim
index 3339adba23a95..2ef0e24541956 100644
--- a/lib/pure/hashes.nim
+++ b/lib/pure/hashes.nim
@@ -182,7 +182,7 @@ proc hashData*(data: pointer, size: int): Hash =
   var h: Hash = 0
   when defined(js):
     var p: cstring
-    asm """`p` = `Data`;"""
+    asm """`p` = `Data`"""
   else:
     var p = cast[cstring](data)
   var i = 0
@@ -193,12 +193,22 @@ proc hashData*(data: pointer, size: int): Hash =
     dec(s)
   result = !$h
 
+proc hashIdentity*[T: Ordinal|enum](x: T): Hash {.inline, since: (1, 3).} =
+  ## The identity hash, i.e. `hashIdentity(x) = x`.
+  cast[Hash](ord(x))
+
+when defined(nimIntHash1):
+  proc hash*[T: Ordinal|enum](x: T): Hash {.inline.} =
+    ## Efficient hashing of integers.
+    cast[Hash](ord(x))
+else:
+  proc hash*[T: Ordinal|enum](x: T): Hash {.inline.} =
+    ## Efficient hashing of integers.
+    hashWangYi1(uint64(ord(x)))
+
 when defined(js):
   var objectID = 0
-
-proc hash*(x: pointer): Hash {.inline.} =
-  ## Efficient hashing of pointers.
-  when defined(js):
+  proc getObjectId(x: pointer): int =
     asm """
       if (typeof `x` == "object") {
         if ("_NimID" in `x`)
@@ -209,29 +219,46 @@ proc hash*(x: pointer): Hash {.inline.} =
         }
       }
     """
+
+proc hash*(x: pointer): Hash {.inline.} =
+  ## Efficient `hash` overload.
+  when defined(js):
+    let y = getObjectId(x)
   else:
-    result = cast[Hash](cast[uint](x) shr 3) # skip the alignment
+    let y = cast[int](x)
+  hash(y) # consistent with code expecting scrambled hashes depending on `nimIntHash1`.
+
+proc hash*[T](x: ref[T] | ptr[T]): Hash {.inline.} =
+  ## Efficient `hash` overload.
+  runnableExamples:
+    var a: array[10, uint8]
+    assert a[0].addr.hash != a[1].addr.hash
+    assert cast[pointer](a[0].addr).hash == a[0].addr.hash
+  runnableExamples:
+    type A = ref object
+      x: int
+    let a = A(x: 3)
+    let ha = a.hash
+    assert ha != A(x: 3).hash # A(x: 3) is a different ref object from `a`.
+    a.x = 4
+    assert ha == a.hash # the hash only depends on the address
+  runnableExamples:
+    # you can overload `hash` if you want to customize semantics
+    type A[T] = ref object
+      x, y: T
+    proc hash(a: A): Hash = hash(a.x)
+    assert A[int](x: 3, y: 4).hash == A[int](x: 3, y: 5).hash
+  # xxx pending bug #17733, merge as `proc hash*(pointer | ref | ptr): Hash`
+  # or `proc hash*[T: ref | ptr](x: T): Hash`
+  hash(cast[pointer](x))
 
 proc hash*[T: proc](x: T): Hash {.inline.} =
   ## Efficient hashing of proc vars. Closures are supported too.
   when T is "closure":
-    result = hash(rawProc(x)) !& hash(rawEnv(x))
+    result = hash((rawProc(x), rawEnv(x)))
   else:
     result = hash(pointer(x))
 
-proc hashIdentity*[T: Ordinal|enum](x: T): Hash {.inline, since: (1, 3).} =
-  ## The identity hash, i.e. `hashIdentity(x) = x`.
-  cast[Hash](ord(x))
-
-when defined(nimIntHash1):
-  proc hash*[T: Ordinal|enum](x: T): Hash {.inline.} =
-    ## Efficient hashing of integers.
-    cast[Hash](ord(x))
-else:
-  proc hash*[T: Ordinal|enum](x: T): Hash {.inline.} =
-    ## Efficient hashing of integers.
-    hashWangYi1(uint64(ord(x)))
-
 proc hash*(x: float): Hash {.inline.} =
   ## Efficient hashing of floats.
   let y = x + 0.0 # for denormalization
@@ -484,10 +511,9 @@ proc hashIgnoreCase*(sBuf: string, sPos, ePos: int): Hash =
     h = h !& ord(c)
   result = !$h
 
-
 proc hash*[T: tuple | object](x: T): Hash =
-  ## Efficient hashing of tuples and objects.
-  ## There must be a `hash` proc defined for each of the field types.
+  ## Efficient `hash` overload.
+  ## `hash` must be defined for each component of `x`.
   runnableExamples:
     type Obj = object
       x: int
diff --git a/testament/lib/stdtest/testutils.nim b/testament/lib/stdtest/testutils.nim
index abffff24c2443..35423de177790 100644
--- a/testament/lib/stdtest/testutils.nim
+++ b/testament/lib/stdtest/testutils.nim
@@ -85,3 +85,7 @@ template accept*(a) =
 
 template reject*(a) =
   doAssert not compiles(a)
+
+template disableVm*(body) =
+  when nimvm: discard
+  else: body
diff --git a/tests/collections/ttables.nim b/tests/collections/ttables.nim
index 61197e9f0edf2..c2864b75f0ff3 100644
--- a/tests/collections/ttables.nim
+++ b/tests/collections/ttables.nim
@@ -7,7 +7,11 @@ And we get here
 3
 '''
 joinable: false
+targets: "c cpp js"
 """
+
+# xxx wrap in a template to test in VM, see https://github.com/timotheecour/Nim/issues/534#issuecomment-769565033
+
 import hashes, sequtils, tables, algorithm
 
 proc sortedPairs[T](t: T): auto = toSeq(t.pairs).sorted
@@ -444,3 +448,13 @@ block emptyOrdered:
   var t2: OrderedTable[int, string]
   doAssert t1 == t2
 
+block: # Table[ref, int]
+  type A = ref object
+    x: int
+  var t: OrderedTable[A, int]
+  let a1 = A(x: 3)
+  let a2 = A(x: 3)
+  t[a1] = 10
+  t[a2] = 11
+  doAssert t[a1] == 10
+  doAssert t[a2] == 11
diff --git a/tests/stdlib/thashes.nim b/tests/stdlib/thashes.nim
index 044259f885d25..66857d3ca72e5 100644
--- a/tests/stdlib/thashes.nim
+++ b/tests/stdlib/thashes.nim
@@ -3,7 +3,7 @@ discard """
 """
 
 import std/hashes
-
+from stdtest/testutils import disableVm, whenVMorJs
 
 when not defined(js) and not defined(cpp):
   block:
@@ -177,5 +177,25 @@ proc main() =
     doAssert hash(Obj5(t: false, x: 1)) == hash(Obj5(t: true, y: 1))
     doAssert hash(Obj5(t: false, x: 1)) != hash(Obj5(t: true, y: 2))
 
+  block: # hash(ref|ptr|pointer)
+    var a: array[10, uint8]
+    # disableVm:
+    whenVMorJs:
+      # pending fix proposed in https://github.com/nim-lang/Nim/issues/15952#issuecomment-786312417
+      discard
+    do:
+      assert a[0].addr.hash != a[1].addr.hash
+      assert cast[pointer](a[0].addr).hash == a[0].addr.hash
+
+  block: # hash(ref)
+    type A = ref object
+      x: int
+    let a = A(x: 3)
+    disableVm: # xxx Error: VM does not support 'cast' from tyRef to tyPointer
+      let ha = a.hash
+      assert ha != A(x: 3).hash # A(x: 3) is a different ref object from `a`.
+      a.x = 4
+      assert ha == a.hash # the hash only depends on the address
+
 static: main()
 main()
diff --git a/tests/stdlib/tstrutils.nim b/tests/stdlib/tstrutils.nim
index a6248d1e3bef5..771dddcafc9d4 100644
--- a/tests/stdlib/tstrutils.nim
+++ b/tests/stdlib/tstrutils.nim
@@ -3,7 +3,7 @@ discard """
 """
 
 import std/strutils
-
+from stdtest/testutils import disableVm
 # xxx each instance of `disableVm` and `when not defined js:` should eventually be fixed
 
 template rejectParse(e) =
@@ -12,10 +12,6 @@ template rejectParse(e) =
     raise newException(AssertionDefect, "This was supposed to fail: $#!" % astToStr(e))
   except ValueError: discard
 
-template disableVm(body) =
-  when nimvm: discard
-  else: body
-
 template main() =
   block: # strip
     doAssert strip("  ha  ") == "ha"