From d6849b87c598b70e83acfee122143ebc2b67df45 Mon Sep 17 00:00:00 2001 From: ReneSac Date: Sun, 12 Jun 2016 16:34:24 -0300 Subject: [PATCH 1/4] Enchanced random access support for queues Now queues support indexing, front() and back() operations and pairs iteration. Also modernized some of the code to use newer Nim features. Added the "add()" alias to "enqueue()", per nim's conventions (also fits better with pop()) --- lib/pure/collections/queues.nim | 150 +++++++++++++++++++++++++++----- 1 file changed, 126 insertions(+), 24 deletions(-) diff --git a/lib/pure/collections/queues.nim b/lib/pure/collections/queues.nim index b9bf33bff..d0fc8f1ca 100644 --- a/lib/pure/collections/queues.nim +++ b/lib/pure/collections/queues.nim @@ -11,6 +11,22 @@ ## Note: For inter thread communication use ## a `Channel `_ instead. +proc englishOrdinal(n: SomeInteger): string = + # Temporary proc. Needs to be moved somewhere else as it can be reused in + # other places too. + # If this accepted number strings instead and only gave out the letters it + # would be more flexible, permitting things like 1.100.000th, 34,545,321st + # but it would be harder and more error prone to use. + let num = $n + if num.len > 1 and num[^2] == '1': + return num & "th" + else: + case num[^1] + of '1': return num & "st" + of '2': return num & "nd" + of '3': return num & "rd" + else: return num & "th" + import math type @@ -20,44 +36,103 @@ type {.deprecated: [TQueue: Queue].} -proc initQueue*[T](initialSize=4): Queue[T] = +proc initQueue*[T](initialSize: int = 4): Queue[T] = ## creates a new queue. `initialSize` needs to be a power of 2. assert isPowerOfTwo(initialSize) result.mask = initialSize-1 newSeq(result.data, initialSize) -proc len*[T](q: Queue[T]): int = +proc len*[T](q: Queue[T]): int {.inline.}= ## returns the number of elements of `q`. result = q.count +proc low*[T](q: Queue[T]): int {.inline.}= + ## returns the index of the oldest element of `q` (always 0). + result = 0 + +proc high*[T](q: Queue[T]): int {.inline.}= + ## returns the index of the last element inserted on `q` (equivalent to + ## `q.len - 1`). + result = q.count - 1 + +proc front*[T](q: Queue[T]): T {.inline.}= + ## returns the oldest element of `q`. Equivalent to `q.pop()` but does not + ## remove it from the queue. + assert q.count > 0 + result = q.data[q.rd] + +proc back*[T](q: Queue[T]): T {.inline.} = + ## returns the newest element of `q` but does not remove it from the queue. + assert q.count > 0 + result = q.data[q.wr - 1] + +template xBoundsCheck(q, i) = + # Bounds check for the array like acceses. + when compileOption("boundChecks"): # d:release should disable this. + if i > q.high: # x < q.low is taken care by the Natural parameter + raise newException(IndexError, + "You tried to access the " & englishOrdinal(i+1) & + " element of the queue but it has only " & + $q.len & " elements.") + discard + +proc `[]`*[T](q: Queue[T], i: Natural) : T {.inline.} = + ## Acess the i-th element of `q` by order of insertion. + ## q[0] is the oldest (the next one q.pop() will extract), + ## q[^1] is the newest (last one added to the queue). + xBoundsCheck(q, i) + return q.data[q.rd + i and q.mask] + +proc `[]`*[T](q: var Queue[T], i: Natural): var T {.inline.} = + ## Acess the i-th element of `q` and returns a mutable + ## reference to it. + xBoundsCheck(q, i) + return q.data[q.rd + i and q.mask] + +proc `[]=`* [T] (q: var Queue[T], i: Natural, val : T) {.inline.} = + ## Change the i-th element of `q`. + xBoundsCheck(q, i) + q.data[q.rd + i and q.mask] = val + iterator items*[T](q: Queue[T]): T = ## yields every element of `q`. var i = q.rd - var c = q.count - while c > 0: - dec c + for c in 0 ..< q.count: yield q.data[i] i = (i + 1) and q.mask iterator mitems*[T](q: var Queue[T]): var T = ## yields every element of `q`. var i = q.rd - var c = q.count - while c > 0: - dec c + for c in 0 ..< q.count: yield q.data[i] i = (i + 1) and q.mask +iterator pairs*[T](q: Queue[T]): tuple[key: int, val: T] = + ## yields every (position, value) of `q`. + var i = q.rd + for c in 0 ..< q.count: + yield (c, q.data[i]) + i = (i + 1) and q.mask + +proc contains*[T](q: Queue[T], item: T): bool {.inline.} = + ## Returns true if `item` is in `q` or false if not found. Usually used + ## via the ``in`` operator. It is the equivalent of ``q.find(item) >= 0``. + ## + ## .. code-block:: Nim + ## if x in q: + ## assert q.contains x + for e in q: + if e == item: return true + return false + proc add*[T](q: var Queue[T], item: T) = ## adds an `item` to the end of the queue `q`. var cap = q.mask+1 if q.count >= cap: - var n: seq[T] - newSeq(n, cap*2) - var i = 0 - for x in items(q): - shallowCopy(n[i], x) - inc i + var n {.noinit.} = newSeq[T](cap*2) + for i, x in q: + shallowCopy(n[i], x) # does not use copyMem because the GC. shallowCopy(q.data, n) q.mask = cap*2 - 1 q.wr = q.count @@ -66,21 +141,25 @@ proc add*[T](q: var Queue[T], item: T) = q.data[q.wr] = item q.wr = (q.wr + 1) and q.mask -proc enqueue*[T](q: var Queue[T], item: T) = - ## alias for the ``add`` operation. - add(q, item) - -proc dequeue*[T](q: var Queue[T]): T = - ## removes and returns the first element of the queue `q`. +proc pop*[T](q: var Queue[T]): T = + ## removes and returns the first (oldest) element of the queue `q`. assert q.count > 0 dec q.count result = q.data[q.rd] q.rd = (q.rd + 1) and q.mask +proc enqueue*[T](q: var Queue[T], item: T) = + ## alias for the ``add`` operation. + q.add(item) + +proc dequeue*[T](q: var Queue[T]): T = + ## alias for the ``pop`` operation. + q.pop() + proc `$`*[T](q: Queue[T]): string = ## turns a queue into its string representation. result = "[" - for x in items(q): + for x in q: if result.len > 1: result.add(", ") result.add($x) result.add("]") @@ -89,14 +168,37 @@ when isMainModule: var q = initQueue[int]() q.add(123) q.add(9) - q.add(4) - var first = q.dequeue + q.enqueue(4) + var first = q.dequeue() q.add(56) q.add(6) - var second = q.dequeue + var second = q.pop() q.add(789) assert first == 123 assert second == 9 assert($q == "[4, 56, 6, 789]") + assert q[0] == q.front and q.front == 4 + assert q[^1] == q.back and q.back == 789 + q[0] = 42 + q[^1] = 7 + assert q[q.low] == 42 + assert q[q.high] == 7 + + assert 6 in q and 789 notin q + assert q.find(6) >= 0 + assert q.find(789) < 0 + + for i in -2 .. 10: + if i in q: + assert q.contains(i) and q.find(i) >= 0 + else: + assert(not q.contains(i) and q.find(i) < 0) + + when compileOption("boundChecks"): + try: + echo q[99] + assert false + except IndexError: + discard From dac4826483f382c62dd10dc2c1b34d03726df780 Mon Sep 17 00:00:00 2001 From: ReneSac Date: Wed, 15 Jun 2016 18:19:51 -0300 Subject: [PATCH 2/4] Improved the documentation and miscelaneous Better bounds checking. Tried to make it and documentation comply with the conflicting style guides. Added example of usage at the top of the module as well as warnings on usage. Also fix the back() and internal englishOrdinal() proc from previous commit. Added {.discardable.} pragma for .pop(), when calling only for it's side effects. Sprinkled some unlikely() for optimization. Some new tests reflecting those changes. --- lib/pure/collections/queues.nim | 137 ++++++++++++++++++++++++-------- 1 file changed, 103 insertions(+), 34 deletions(-) diff --git a/lib/pure/collections/queues.nim b/lib/pure/collections/queues.nim index d0fc8f1ca..96c6c75d4 100644 --- a/lib/pure/collections/queues.nim +++ b/lib/pure/collections/queues.nim @@ -8,6 +8,33 @@ # ## Implementation of a `queue`:idx:. The underlying implementation uses a ``seq``. +## +## None of the procs that get an individual value from the queue can be used +## on an empty queue. +## If compiled with `boundChecks` option, those procs will raise an `IndexError` +## on such access. This should not be relied upon, as `-d:release` will +## disable those checks and may return garbage or crash the program. +## +## As such, a check to see if the queue is empty is needed before any +## access, unless your program logic guarantees it indirectly. +## +## .. code-block:: Nim +## proc foo(a, b: Positive) = # assume random positive values for `a` and `b` +## var q = initQueue[int]() # initializes the object +## for i in 1 ..< a: q.add i # populates the queue +## +## if b < q.len: # checking before indexed access +## echo "The element at index position ", b, " is ", q[b] +## +## # The following two lines don't need any checking on access due to the +## # logic of the program, but that would not be the case if `a` could be 0. +## assert q.front == 1 +## assert q.back == a +## +## while q.len > 0: # checking if the queue is empty +## echo q.pop() +## +## ## Note: For inter thread communication use ## a `Channel `_ instead. @@ -30,61 +57,71 @@ proc englishOrdinal(n: SomeInteger): string = import math type - Queue*[T] = object ## a queue + Queue*[T] = object ## A queue. data: seq[T] rd, wr, count, mask: int {.deprecated: [TQueue: Queue].} proc initQueue*[T](initialSize: int = 4): Queue[T] = - ## creates a new queue. `initialSize` needs to be a power of 2. + ## Create a new queue. + ## Optionally, the initial capacity can be reserved via `initialSize` as a + ## performance optimization. `initialSize` needs to be a power of 2. + ## The lenght of a newly created queue will still be 0. assert isPowerOfTwo(initialSize) result.mask = initialSize-1 newSeq(result.data, initialSize) proc len*[T](q: Queue[T]): int {.inline.}= - ## returns the number of elements of `q`. + ## Return the number of elements of `q`. result = q.count proc low*[T](q: Queue[T]): int {.inline.}= - ## returns the index of the oldest element of `q` (always 0). + ## Return the index of the oldest element of `q` (always 0). result = 0 proc high*[T](q: Queue[T]): int {.inline.}= - ## returns the index of the last element inserted on `q` (equivalent to + ## Return the index of the last element inserted on `q` (equivalent to ## `q.len - 1`). result = q.count - 1 +template emptyCheck(q) = + # Bounds check for the regular queue access. + when compileOption("boundChecks"): + if unlikely(q.count < 1): + raise newException(IndexError, "Empty queue.") + discard + +template xBoundsCheck(q, i) = + # Bounds check for the array like accesses. + when compileOption("boundChecks"): # d:release should disable this. + if unlikely(i >= q.count): # x < q.low is taken care by the Natural parameter + raise newException(IndexError, + "Tried to access the " & englishOrdinal(i+1) & + " element of the queue but it has only " & + $q.count & " elements.") + discard + proc front*[T](q: Queue[T]): T {.inline.}= - ## returns the oldest element of `q`. Equivalent to `q.pop()` but does not + ## Return the oldest element of `q`. Equivalent to `q.pop()` but does not ## remove it from the queue. - assert q.count > 0 + emptyCheck(q) result = q.data[q.rd] proc back*[T](q: Queue[T]): T {.inline.} = - ## returns the newest element of `q` but does not remove it from the queue. - assert q.count > 0 - result = q.data[q.wr - 1] - -template xBoundsCheck(q, i) = - # Bounds check for the array like acceses. - when compileOption("boundChecks"): # d:release should disable this. - if i > q.high: # x < q.low is taken care by the Natural parameter - raise newException(IndexError, - "You tried to access the " & englishOrdinal(i+1) & - " element of the queue but it has only " & - $q.len & " elements.") - discard + ## Return the newest element of `q` but does not remove it from the queue. + emptyCheck(q) + result = q.data[q.wr - 1 and q.mask] proc `[]`*[T](q: Queue[T], i: Natural) : T {.inline.} = - ## Acess the i-th element of `q` by order of insertion. + ## Access the i-th element of `q` by order of insertion. ## q[0] is the oldest (the next one q.pop() will extract), ## q[^1] is the newest (last one added to the queue). xBoundsCheck(q, i) return q.data[q.rd + i and q.mask] proc `[]`*[T](q: var Queue[T], i: Natural): var T {.inline.} = - ## Acess the i-th element of `q` and returns a mutable + ## Access the i-th element of `q` and returns a mutable ## reference to it. xBoundsCheck(q, i) return q.data[q.rd + i and q.mask] @@ -95,28 +132,28 @@ proc `[]=`* [T] (q: var Queue[T], i: Natural, val : T) {.inline.} = q.data[q.rd + i and q.mask] = val iterator items*[T](q: Queue[T]): T = - ## yields every element of `q`. + ## Yield every element of `q`. var i = q.rd for c in 0 ..< q.count: yield q.data[i] i = (i + 1) and q.mask iterator mitems*[T](q: var Queue[T]): var T = - ## yields every element of `q`. + ## Yield every element of `q`. var i = q.rd for c in 0 ..< q.count: yield q.data[i] i = (i + 1) and q.mask iterator pairs*[T](q: Queue[T]): tuple[key: int, val: T] = - ## yields every (position, value) of `q`. + ## Yield every (position, value) of `q`. var i = q.rd for c in 0 ..< q.count: yield (c, q.data[i]) i = (i + 1) and q.mask proc contains*[T](q: Queue[T], item: T): bool {.inline.} = - ## Returns true if `item` is in `q` or false if not found. Usually used + ## Return true if `item` is in `q` or false if not found. Usually used ## via the ``in`` operator. It is the equivalent of ``q.find(item) >= 0``. ## ## .. code-block:: Nim @@ -127,9 +164,9 @@ proc contains*[T](q: Queue[T], item: T): bool {.inline.} = return false proc add*[T](q: var Queue[T], item: T) = - ## adds an `item` to the end of the queue `q`. + ## Add an `item` to the end of the queue `q`. var cap = q.mask+1 - if q.count >= cap: + if unlikely(q.count >= cap): var n {.noinit.} = newSeq[T](cap*2) for i, x in q: shallowCopy(n[i], x) # does not use copyMem because the GC. @@ -141,23 +178,23 @@ proc add*[T](q: var Queue[T], item: T) = q.data[q.wr] = item q.wr = (q.wr + 1) and q.mask -proc pop*[T](q: var Queue[T]): T = - ## removes and returns the first (oldest) element of the queue `q`. - assert q.count > 0 +proc pop*[T](q: var Queue[T]): T {.inline, discardable.} = + ## Remove and returns the first (oldest) element of the queue `q`. + emptyCheck(q) dec q.count result = q.data[q.rd] q.rd = (q.rd + 1) and q.mask proc enqueue*[T](q: var Queue[T], item: T) = - ## alias for the ``add`` operation. + ## Alias for the ``add`` operation. q.add(item) proc dequeue*[T](q: var Queue[T]): T = - ## alias for the ``pop`` operation. + ## Alias for the ``pop`` operation. q.pop() proc `$`*[T](q: Queue[T]): string = - ## turns a queue into its string representation. + ## Turn a queue into its string representation. result = "[" for x in q: if result.len > 1: result.add(", ") @@ -202,3 +239,35 @@ when isMainModule: assert false except IndexError: discard + + try: + assert q.len == 4 + for i in 0 ..< 5: q.pop() + assert false + except IndexError: + discard + + # Similar to proc from the documentation example + proc foo(a, b: Positive) = # assume random positive values for `a` and `b`. + var q = initQueue[int]() + assert q.len == 0 + for i in 1 .. a: q.add i + + if b < q.len: # checking before indexed access. + assert q[b] == b + 1 + + # The following two lines don't need any checking on access due to the logic + # of the program, but that would not be the case if `a` could be 0. + assert q.front == 1 + assert q.back == a + + while q.len > 0: # checking if the queue is empty + assert q.pop() > 0 + + #foo(0,0) + foo(8,5) + foo(10,9) + foo(1,1) + foo(2,1) + foo(1,5) + foo(3,2) From 8dcb3fe5b7fd829ff9de6a7a051cca6f94d4efb0 Mon Sep 17 00:00:00 2001 From: ReneSac Date: Thu, 16 Jun 2016 17:33:45 -0300 Subject: [PATCH 3/4] Fixes for things pointed by Araq on the PR --- lib/pure/collections/queues.nim | 45 +++++++++++++-------------------- 1 file changed, 18 insertions(+), 27 deletions(-) diff --git a/lib/pure/collections/queues.nim b/lib/pure/collections/queues.nim index 96c6c75d4..5ec1d05bf 100644 --- a/lib/pure/collections/queues.nim +++ b/lib/pure/collections/queues.nim @@ -34,26 +34,9 @@ ## while q.len > 0: # checking if the queue is empty ## echo q.pop() ## -## ## Note: For inter thread communication use ## a `Channel `_ instead. -proc englishOrdinal(n: SomeInteger): string = - # Temporary proc. Needs to be moved somewhere else as it can be reused in - # other places too. - # If this accepted number strings instead and only gave out the letters it - # would be more flexible, permitting things like 1.100.000th, 34,545,321st - # but it would be harder and more error prone to use. - let num = $n - if num.len > 1 and num[^2] == '1': - return num & "th" - else: - case num[^1] - of '1': return num & "st" - of '2': return num & "nd" - of '3': return num & "rd" - else: return num & "th" - import math type @@ -66,8 +49,12 @@ type proc initQueue*[T](initialSize: int = 4): Queue[T] = ## Create a new queue. ## Optionally, the initial capacity can be reserved via `initialSize` as a - ## performance optimization. `initialSize` needs to be a power of 2. - ## The lenght of a newly created queue will still be 0. + ## performance optimization. The length of a newly created queue will still + ## be 0. + ## + ## `initialSize` needs to be a power of two. If you need to accept runtime + ## values for this you could use the ``nextPowerOfTwo`` proc from the + ## `math `_ module. assert isPowerOfTwo(initialSize) result.mask = initialSize-1 newSeq(result.data, initialSize) @@ -90,17 +77,13 @@ template emptyCheck(q) = when compileOption("boundChecks"): if unlikely(q.count < 1): raise newException(IndexError, "Empty queue.") - discard template xBoundsCheck(q, i) = # Bounds check for the array like accesses. when compileOption("boundChecks"): # d:release should disable this. if unlikely(i >= q.count): # x < q.low is taken care by the Natural parameter raise newException(IndexError, - "Tried to access the " & englishOrdinal(i+1) & - " element of the queue but it has only " & - $q.count & " elements.") - discard + "Out of bounds: " & $i & " > " & $(q.count - 1)) proc front*[T](q: Queue[T]): T {.inline.}= ## Return the oldest element of `q`. Equivalent to `q.pop()` but does not @@ -167,7 +150,7 @@ proc add*[T](q: var Queue[T], item: T) = ## Add an `item` to the end of the queue `q`. var cap = q.mask+1 if unlikely(q.count >= cap): - var n {.noinit.} = newSeq[T](cap*2) + var n = newSeq[T](cap*2) for i, x in q: shallowCopy(n[i], x) # does not use copyMem because the GC. shallowCopy(q.data, n) @@ -196,13 +179,13 @@ proc dequeue*[T](q: var Queue[T]): T = proc `$`*[T](q: Queue[T]): string = ## Turn a queue into its string representation. result = "[" - for x in q: + for x in items(q): # Don't remove the items here for reasons that don't fit in this margin. if result.len > 1: result.add(", ") result.add($x) result.add("]") when isMainModule: - var q = initQueue[int]() + var q = initQueue[int](1) q.add(123) q.add(9) q.enqueue(4) @@ -247,6 +230,14 @@ when isMainModule: except IndexError: discard + # grabs some types of resize error. + q = initQueue[int]() + for i in 1 .. 4: q.add i + q.pop() + q.pop() + for i in 5 .. 8: q.add i + assert $q == "[3, 4, 5, 6, 7, 8]" + # Similar to proc from the documentation example proc foo(a, b: Positive) = # assume random positive values for `a` and `b`. var q = initQueue[int]() From 67c7a925c1de5ecf0981239dea1bca3ee114de26 Mon Sep 17 00:00:00 2001 From: ReneSac Date: Thu, 16 Jun 2016 18:08:15 -0300 Subject: [PATCH 4/4] Remove high() and low() procs from queues module Just in case as they are said not overloadable. No deprecation because this is during a PR: those procs didn't exist before. Also update comment due to failed optimization attempt using copyMem() for POD datatypes. --- lib/pure/collections/queues.nim | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/lib/pure/collections/queues.nim b/lib/pure/collections/queues.nim index 5ec1d05bf..911816518 100644 --- a/lib/pure/collections/queues.nim +++ b/lib/pure/collections/queues.nim @@ -63,15 +63,6 @@ proc len*[T](q: Queue[T]): int {.inline.}= ## Return the number of elements of `q`. result = q.count -proc low*[T](q: Queue[T]): int {.inline.}= - ## Return the index of the oldest element of `q` (always 0). - result = 0 - -proc high*[T](q: Queue[T]): int {.inline.}= - ## Return the index of the last element inserted on `q` (equivalent to - ## `q.len - 1`). - result = q.count - 1 - template emptyCheck(q) = # Bounds check for the regular queue access. when compileOption("boundChecks"): @@ -151,8 +142,8 @@ proc add*[T](q: var Queue[T], item: T) = var cap = q.mask+1 if unlikely(q.count >= cap): var n = newSeq[T](cap*2) - for i, x in q: - shallowCopy(n[i], x) # does not use copyMem because the GC. + for i, x in q: # don't use copyMem because the GC and because it's slower. + shallowCopy(n[i], x) shallowCopy(q.data, n) q.mask = cap*2 - 1 q.wr = q.count @@ -203,8 +194,6 @@ when isMainModule: assert q[^1] == q.back and q.back == 789 q[0] = 42 q[^1] = 7 - assert q[q.low] == 42 - assert q[q.high] == 7 assert 6 in q and 789 notin q assert q.find(6) >= 0