From edb5a1a5c6b8381c41561f303aa3588ff5fbab69 Mon Sep 17 00:00:00 2001 From: Kier Davis Date: Sat, 18 Jun 2016 15:43:32 +0100 Subject: Fix clear() on CountTable The record tuples used in CountData.data don't contain an 'hcode' member, unlike Table and OrderedTable, causing the existing clearImpl() implementation to break when attempting to assign to t.data[i].hcode. --- lib/pure/collections/tableimpl.nim | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/pure/collections/tableimpl.nim b/lib/pure/collections/tableimpl.nim index 1bbf19ee9..f73632008 100644 --- a/lib/pure/collections/tableimpl.nim +++ b/lib/pure/collections/tableimpl.nim @@ -142,7 +142,8 @@ template delImpl() {.dirty, immediate.} = template clearImpl() {.dirty, immediate.} = for i in 0 .. Date: Sat, 18 Jun 2016 15:45:31 +0100 Subject: Add tests for tables.clear() This should reduce the chance of regressions. --- tests/collections/ttables.nim | 21 +++++++++++++++++++++ tests/collections/ttablesref.nim | 25 +++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/tests/collections/ttables.nim b/tests/collections/ttables.nim index a8a182a78..773a8f3b7 100644 --- a/tests/collections/ttables.nim +++ b/tests/collections/ttables.nim @@ -134,6 +134,27 @@ block mpairsTableTest1: block SyntaxTest: var x = toTable[int, string]({:}) +block clearTableTest: + var t = data.toTable + assert t.len() != 0 + t.clear() + assert t.len() == 0 + +block clearOrderedTableTest: + var t = data.toOrderedTable + assert t.len() != 0 + t.clear() + assert t.len() == 0 + +block clearCountTableTest: + var t = initCountTable[string]() + t.inc("90", 3) + t.inc("12", 2) + t.inc("34", 1) + assert t.len() != 0 + t.clear() + assert t.len() == 0 + proc orderedTableSortTest() = var t = initOrderedTable[string, int](2) for key, val in items(data): t[key] = val diff --git a/tests/collections/ttablesref.nim b/tests/collections/ttablesref.nim index 32494f1f2..12af1ccbb 100644 --- a/tests/collections/ttablesref.nim +++ b/tests/collections/ttablesref.nim @@ -141,6 +141,31 @@ block anonZipTest: let values = @[1, 2, 3] doAssert "{a: 1, b: 2, c: 3}" == $ toTable zip(keys, values) +block clearTableTest: + var t = newTable[string, float]() + t["test"] = 1.2345 + t["111"] = 1.000043 + t["123"] = 1.23 + assert t.len() != 0 + t.clear() + assert t.len() == 0 + +block clearOrderedTableTest: + var t = newOrderedTable[string, int](2) + for key, val in items(data): t[key] = val + assert t.len() != 0 + t.clear() + assert t.len() == 0 + +block clearCountTableTest: + var t = newCountTable[string]() + t.inc("90", 3) + t.inc("12", 2) + t.inc("34", 1) + assert t.len() != 0 + t.clear() + assert t.len() == 0 + orderedTableSortTest() echo "true" -- cgit 1.4.1-2-gfad0 From 449960bf7e3cad1aa4f3e38e39582ac220237ecb Mon Sep 17 00:00:00 2001 From: Kier Davis Date: Sat, 9 Jul 2016 17:34:01 +0100 Subject: Add a fix for clear() on non-ref types by adding a missing 'var' annotation to the type signature However, this fix won't take effect until a compiler bug (#4448) is fixed. Until then, the codebase functions identically to how it did before this commit (calls to clear() fail to compile for Table/OrderedTable/CountTable as the argument is immutable). --- lib/pure/collections/tables.nim | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/pure/collections/tables.nim b/lib/pure/collections/tables.nim index e454a43cb..3a0c50c50 100644 --- a/lib/pure/collections/tables.nim +++ b/lib/pure/collections/tables.nim @@ -85,7 +85,7 @@ template dataLen(t): expr = len(t.data) include tableimpl -proc clear*[A, B](t: Table[A, B] | TableRef[A, B]) = +proc clear*[A, B](t: var Table[A, B] | TableRef[A, B]) = ## Resets the table so that it is empty. clearImpl() @@ -425,7 +425,7 @@ proc len*[A, B](t: OrderedTable[A, B]): int {.inline.} = ## returns the number of keys in `t`. result = t.counter -proc clear*[A, B](t: OrderedTable[A, B] | OrderedTableRef[A, B]) = +proc clear*[A, B](t: var OrderedTable[A, B] | OrderedTableRef[A, B]) = ## Resets the table so that it is empty. clearImpl() t.first = -1 @@ -754,7 +754,7 @@ proc len*[A](t: CountTable[A]): int = ## returns the number of keys in `t`. result = t.counter -proc clear*[A](t: CountTable[A] | CountTableRef[A]) = +proc clear*[A](t: var CountTable[A] | CountTableRef[A]) = ## Resets the table so that it is empty. clearImpl() t.counter = 0 -- cgit 1.4.1-2-gfad0 From 8e843354e12fdaf7697cfdfe9cd4efd83737db18 Mon Sep 17 00:00:00 2001 From: Kier Davis Date: Sat, 9 Jul 2016 18:21:37 +0100 Subject: Disable failing tests for tables.clear() The tests for tables.clear() in tests/collections/ttables.nim currently fail as a result of #4448, so I've wrapped them in a 'when false' to disable them until the bug is fixed. --- tests/collections/ttables.nim | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/tests/collections/ttables.nim b/tests/collections/ttables.nim index 773a8f3b7..59fef4920 100644 --- a/tests/collections/ttables.nim +++ b/tests/collections/ttables.nim @@ -134,26 +134,28 @@ block mpairsTableTest1: block SyntaxTest: var x = toTable[int, string]({:}) -block clearTableTest: - var t = data.toTable - assert t.len() != 0 - t.clear() - assert t.len() == 0 - -block clearOrderedTableTest: - var t = data.toOrderedTable - assert t.len() != 0 - t.clear() - assert t.len() == 0 - -block clearCountTableTest: - var t = initCountTable[string]() - t.inc("90", 3) - t.inc("12", 2) - t.inc("34", 1) - assert t.len() != 0 - t.clear() - assert t.len() == 0 +# Until #4448 is fixed, these tests will fail +when false: + block clearTableTest: + var t = data.toTable + assert t.len() != 0 + t.clear() + assert t.len() == 0 + + block clearOrderedTableTest: + var t = data.toOrderedTable + assert t.len() != 0 + t.clear() + assert t.len() == 0 + + block clearCountTableTest: + var t = initCountTable[string]() + t.inc("90", 3) + t.inc("12", 2) + t.inc("34", 1) + assert t.len() != 0 + t.clear() + assert t.len() == 0 proc orderedTableSortTest() = var t = initOrderedTable[string, int](2) -- cgit 1.4.1-2-gfad0