From 65f3b53a56869848aa85c467bfd2cb0a5ebd9c5b Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Tue, 25 Feb 2020 18:39:54 -0800 Subject: [PATCH] fix #13504; add SharedTable tests --- lib/pure/collections/setimpl.nim | 6 +- lib/pure/collections/sharedtables.nim | 5 ++ lib/pure/collections/tableimpl.nim | 22 +++++-- lib/pure/collections/tables.nim | 4 ++ tests/sets/tsets_various.nim | 29 +++++++++ tests/stdlib/tsharedtable.nim | 86 +++++++++++++++++++++++++-- 6 files changed, 137 insertions(+), 15 deletions(-) diff --git a/lib/pure/collections/setimpl.nim b/lib/pure/collections/setimpl.nim index f8950e3546b3e..f7a48ab911013 100644 --- a/lib/pure/collections/setimpl.nim +++ b/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 diff --git a/lib/pure/collections/sharedtables.nim b/lib/pure/collections/sharedtables.nim index 0fbbdb3eb0e31..27ac5e84f7e3b 100644 --- a/lib/pure/collections/sharedtables.nim +++ b/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. ## diff --git a/lib/pure/collections/tableimpl.nim b/lib/pure/collections/tableimpl.nim index aabaeeeb37968..d7facda7209aa 100644 --- a/lib/pure/collections/tableimpl.nim +++ b/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() diff --git a/lib/pure/collections/tables.nim b/lib/pure/collections/tables.nim index cbe7578447ef9..131804a2241c3 100644 --- a/lib/pure/collections/tables.nim +++ b/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 diff --git a/tests/sets/tsets_various.nim b/tests/sets/tsets_various.nim index 4fd602510cd97..c27d8e124caae 100644 --- a/tests/sets/tsets_various.nim +++ b/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 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]) diff --git a/tests/stdlib/tsharedtable.nim b/tests/stdlib/tsharedtable.nim index 99d20e08a38cd..ce6aa96df84d2 100644 --- a/tests/stdlib/tsharedtable.nim +++ b/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..