fix nim-lang#13504; add SharedTable tests

lib/pure/collections/setimpl.nim

@@ -38,7 +38,7 @@ proc enlarge[A](s: var HashSet[A]) =
   newSeq(n, len(s.data) * growthFactor)
   swap(s.data, n) # n is now old seq
   for i in countup(0, high(n)):
-    if isFilled(n[i].hcode):
+    if isFilledAndValid(n[i].hcode):
       var j = -1 - rawGetKnownHC(s, n[i].key, n[i].hcode)
       rawInsert(s, s.data, n[i].key, n[i].hcode, j)
 
@@ -112,7 +112,7 @@ proc enlarge[A](s: var OrderedSet[A]) =
   swap(s.data, n)
   while h >= 0:
     var nxt = n[h].next
-    if isFilled(n[h].hcode):
+    if isFilled(n[h].hcode): # should be isFilledAndValid once tombstones are used
       var j = -1 - rawGetKnownHC(s, n[h].key, n[h].hcode)
       rawInsert(s, s.data, n[h].key, n[h].hcode, j)
     h = nxt
@@ -130,7 +130,7 @@ proc exclImpl[A](s: var OrderedSet[A], key: A): bool {.inline.} =
   result = true
   while h >= 0:
     var nxt = n[h].next
-    if isFilled(n[h].hcode):
+    if isFilled(n[h].hcode): # should be isFilledAndValid once tombstones are used
       if n[h].hcode == hc and n[h].key == key:
         dec s.counter
         result = false

lib/pure/collections/sharedtables.nim

@@ -206,6 +206,11 @@ proc del*[A, B](t: var SharedTable[A, B], key: A) =
   withLock t:
     delImpl()
 
+proc len*[A, B](t: var SharedTable[A, B]): int =
+  ## number of elements in `t`
+  withLock t:
+    result = t.counter
+
 proc init*[A, B](t: var SharedTable[A, B], initialSize = 64) =
   ## creates a new hash table that is empty.
   ##

lib/pure/collections/tableimpl.nim

@@ -107,13 +107,23 @@ template clearImpl() {.dirty.} =
     t.data[i].val = default(type(t.data[i].val))
   t.counter = 0
 
+template ctAnd(a, b): bool =
+  # pending https://github.com/nim-lang/Nim/issues/13502
+  when a:
+    when b: true
+    else: false
+  else: false
+
 template initImpl(result: typed, size: int) =
-  assert isPowerOfTwo(size)
-  result.counter = 0
-  newSeq(result.data, size)
-  when compiles(result.first):
-    result.first = -1
-    result.last = -1
+  when ctAnd(declared(SharedTable), type(result) is SharedTable):
+    init(result, size)
+  else:
+    assert isPowerOfTwo(size)
+    result.counter = 0
+    newSeq(result.data, size)
+    when compiles(result.first):
+      result.first = -1
+      result.last = -1
 
 template insertImpl() = # for CountTable
   checkIfInitialized()

lib/pure/collections/tables.nim

@@ -1282,6 +1282,10 @@ template forAllOrderedPairs(yieldStmt: untyped) {.dirty.} =
     var h = t.first
     while h >= 0:
       var nxt = t.data[h].next
+       # For OrderedTable/OrderedTableRef, isFilled is ok because `del` is O(n)
+       # and doesn't create tombsones, but if it does start using tombstones,
+       # carefully replace `isFilled` by `isFilledAndValid` as appropriate for these
+       # table types only, ditto with `OrderedSet`.
       if isFilled(t.data[h].hcode):
         yieldStmt
       h = nxt

tests/sets/tsets_various.nim

@@ -225,3 +225,32 @@ block: # https://github.com/nim-lang/Nim/issues/13496
 
   testDel(): (var t: HashSet[int])
   testDel(): (var t: OrderedSet[int])
+
+block: # test correctness after a number of inserts/deletes
+  template testDel(body) =
+    block:
+      body
+      var expected: seq[int]
+      let n = 100
+      let n2 = n*2
+      for i in 0..<n:
+        t.incl(i)
+      for i in 0..<n:
+        if i mod 3 == 0:
+          t.excl(i)
+      for i in n..<n2:
+        t.incl(i)
+      for i in 0..<n2:
+        if i mod 7 == 0:
+          t.excl(i)
+
+      for i in 0..<n2:
+        if (i>=n or i mod 3 != 0) and i mod 7 != 0:
+          expected.add i
+
+      for i in expected: doAssert i in t
+      doAssert t.len == expected.len
+      doAssert sortedItems(t) == expected
+
+  testDel(): (var t: HashSet[int])
+  testDel(): (var t: OrderedSet[int])

tests/stdlib/tsharedtable.nim

@@ -6,10 +6,84 @@ output: '''
 
 import sharedtables
 
-var table: SharedTable[int, int]
+block:
+  var table: SharedTable[int, int]
 
-init(table)
-table[1] = 10
-assert table.mget(1) == 10
-assert table.mgetOrPut(3, 7) == 7
-assert table.mgetOrPut(3, 99) == 7
+  init(table)
+  table[1] = 10
+  assert table.mget(1) == 10
+  assert table.mgetOrPut(3, 7) == 7
+  assert table.mgetOrPut(3, 99) == 7
+  deinitSharedTable(table)
+
+import sequtils, algorithm
+proc sortedPairs[T](t: T): auto = toSeq(t.pairs).sorted
+template sortedItems(t: untyped): untyped = sorted(toSeq(t))
+
+import tables # refs issue #13504
+
+block: # we use Table as groundtruth, it's well tested elsewhere
+  template testDel(t, t0) =
+    template put2(i) =
+      t[i] = i
+      t0[i] = i
+
+    template add2(i, val) =
+      t.add(i, val)
+      t0.add(i, val)
+
+    template del2(i) =
+      t.del(i)
+      t0.del(i)
+
+    template checkEquals() =
+      doAssert t.len == t0.len
+      for k,v in t0:
+        doAssert t.mgetOrPut(k, -1) == v # sanity check
+        doAssert t.mget(k) == v
+
+    let n = 100
+    let n2 = n*2
+    let n3 = n*3
+    let n4 = n*4
+    let n5 = n*5
+
+    for i in 0..<n:
+      put2(i)
+    for i in 0..<n:
+      if i mod 3 == 0:
+        del2(i)
+    for i in n..<n2:
+      put2(i)
+    for i in 0..<n2:
+      if i mod 7 == 0:
+        del2(i)
+
+    checkEquals()
+
+    for i in n2..<n3:
+      t0[i] = -2
+      doAssert t.mgetOrPut(i, -2) == -2
+      doAssert t.mget(i) == -2
+
+    for i in 0..<n4:
+      let ok = i in t0
+      if not ok: t0[i] = -i
+      doAssert t.hasKeyOrPut(i, -i) == ok
+
+    checkEquals()
+
+    for i in n4..<n5:
+      add2(i, i*10)
+      add2(i, i*11)
+      add2(i, i*12)
+      del2(i)
+      del2(i)
+
+    checkEquals()
+
+  var t: SharedTable[int, int]
+  init(t) # ideally should be auto-init
+  var t0: Table[int, int]
+  testDel(t, t0)
+  deinitSharedTable(t)