Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std/lists: O(1) concatenation of singly- and doubly linked lists. #16362

Merged
merged 35 commits into from
Dec 20, 2020
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
07bd412
O(1) concatenation of singly- and doubly linked lists.
salvipeter Dec 15, 2020
f9b5ead
Add `sequtils` import to runnable examples with `toSeq`.
salvipeter Dec 15, 2020
5bfc8c3
Added missing call to runnable examples.
salvipeter Dec 15, 2020
dd8a160
Add .since annotation, changelog, and tests.
salvipeter Dec 15, 2020
f1b76c7
Rename `lists.concat` as an overload to `lists.append`.
salvipeter Dec 15, 2020
05654b1
Renamed `append` to `add` in lists.
salvipeter Dec 16, 2020
72b07e0
Remove faulty `add` for `DoublyLinkedList`s.
salvipeter Dec 16, 2020
77496b5
Improved tests for lists.
salvipeter Dec 17, 2020
d3350de
`lists.add` moves the second list; added `lists.copy` for shallow copy.
salvipeter Dec 17, 2020
50d75f6
More tests for `lists.add` and `lists.copy`.
salvipeter Dec 17, 2020
18d27cc
More compact tests for lists with templates.
salvipeter Dec 17, 2020
d29b0fe
List concatenation with copying (`add`) and moving (tentatively `addM…
salvipeter Dec 17, 2020
6e2cea2
Renamed `addMove` to `addMoved`; renamed arguments according to the s…
salvipeter Dec 18, 2020
1d2f8a1
Added extended example to `lists.copy`.
salvipeter Dec 18, 2020
533c1c0
Corrected .since annotations to 1.6
salvipeter Dec 18, 2020
bd2c84d
Add .since annotation, changelog, and tests.
salvipeter Dec 15, 2020
7022eaf
Rename `lists.concat` as an overload to `lists.append`.
salvipeter Dec 15, 2020
cc53d47
Renamed `append` to `add` in lists.
salvipeter Dec 16, 2020
7cc0d42
Remove faulty `add` for `DoublyLinkedList`s.
salvipeter Dec 16, 2020
192ca4b
`lists.add` moves the second list; added `lists.copy` for shallow copy.
salvipeter Dec 17, 2020
3cf789b
More tests for `lists.add` and `lists.copy`.
salvipeter Dec 17, 2020
e18e9a0
List concatenation with copying (`add`) and moving (tentatively `addM…
salvipeter Dec 17, 2020
96274cb
Renamed `addMove` to `addMoved`; renamed arguments according to the s…
salvipeter Dec 18, 2020
71530a7
Since declarations changed to (1,5,1).
salvipeter Dec 18, 2020
74b4aa3
Add .since annotation, changelog, and tests.
salvipeter Dec 15, 2020
363571d
Rename `lists.concat` as an overload to `lists.append`.
salvipeter Dec 15, 2020
e20d684
Renamed `append` to `add` in lists.
salvipeter Dec 16, 2020
1495139
Remove faulty `add` for `DoublyLinkedList`s.
salvipeter Dec 16, 2020
a9e0b5f
`lists.add` moves the second list; added `lists.copy` for shallow copy.
salvipeter Dec 17, 2020
a4fba89
More tests for `lists.add` and `lists.copy`.
salvipeter Dec 17, 2020
7538244
List concatenation with copying (`add`) and moving (tentatively `addM…
salvipeter Dec 17, 2020
37f6299
Renamed `addMove` to `addMoved`; renamed arguments according to the s…
salvipeter Dec 18, 2020
7cb28fe
Changelog update.
salvipeter Dec 18, 2020
7eb491a
Fix rebasing errors.
salvipeter Dec 18, 2020
5305d46
Self-adding with `lists.addMove` results in a cycle now.
salvipeter Dec 19, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,27 @@

- `echo` and `debugEcho` will now raise `IOError` if writing to stdout fails. Previous behavior
silently ignored errors. See #16366. Use `-d:nimLegacyEchoNoRaise` for previous behavior.
- Added `lists.toSinglyLinkedList` and `lists.toDoublyLinkedList` for conversion from
Copy link
Member

@timotheecour timotheecour Dec 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • changelog needs to be fixed (reason for duplication is /changelog.md merge=union, but it's there for a good reason)

  • group everything for this PR under just 1 entry

  • newline between preceding entry (echo and debugEcho...) and your new entry

`openArray`s.

- Added `lists.copy` for shallow copying singly- and doubly linked lists.

- `add` is now overloaded also for the concatenation of singly- and doubly linked lists;
an O(1) variation that consumes its argument, `addMoved`, is also supplied.
- Added `lists.toSinglyLinkedList` and `lists.toDoublyLinkedList` for conversion from
`openArray`s.

- Added `lists.copy` for shallow copying singly- and doubly linked lists.

- `add` is now overloaded also for the concatenation of singly- and doubly linked lists;
an O(1) variation that consumes its argument, `addMoved`, is also supplied.
- Added `lists.toSinglyLinkedList` and `lists.toDoublyLinkedList` for conversion from
`openArray`s.

- Added `lists.copy` for shallow copying singly- and doubly linked lists.

- `add` is now overloaded also for the concatenation of singly- and doubly linked lists;
an O(1) variation that consumes its argument, `addMoved`, is also supplied.

## Language changes

Expand Down
193 changes: 192 additions & 1 deletion lib/pure/collections/lists.nim
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
## * `deques module <deques.html>`_ for double-ended queues
## * `sharedlist module <sharedlist.html>`_ for shared singly-linked lists

import std/private/since

when not defined(nimhygiene):
{.pragma: dirty.}
Expand Down Expand Up @@ -178,6 +179,28 @@ proc newSinglyLinkedNode*[T](value: T): <//>(SinglyLinkedNode[T]) =
new(result)
result.value = value

func toSinglyLinkedList*[T](elems: openArray[T]): SinglyLinkedList[T] {.since: (1, 5, 1).} =
## Creates a new `SinglyLinkedList` with the members of the
## given collection (seq, array, or string) `elems`.
Copy link
Member

@timotheecour timotheecour Dec 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's not the place to describe what openArray is; it would violate "one definition rule" for docs; note that openArray definition is 1 click away thanks to docgen
=> simply:

Creates a new `SinglyLinkedList` from members of `elems`

(+ elsewhere)

runnableExamples:
import sequtils
let a = [1, 2, 3, 4, 5].toSinglyLinkedList
assert a.toSeq == [1, 2, 3, 4, 5]
result = initSinglyLinkedList[T]()
for elem in elems.items:
result.append(elem)

func toDoublyLinkedList*[T](elems: openArray[T]): DoublyLinkedList[T] {.since: (1, 5, 1).} =
## Creates a new `DoublyLinkedList` with the members of the
## given collection (seq, array, or string) `elems`.
runnableExamples:
import sequtils
let a = [1, 2, 3, 4, 5].toDoublyLinkedList
assert a.toSeq == [1, 2, 3, 4, 5]
result = initDoublyLinkedList[T]()
for elem in elems.items:
result.append(elem)

template itemsListImpl() {.dirty.} =
var it = L.head
while it != nil:
Expand Down Expand Up @@ -435,7 +458,81 @@ proc prepend*[T](L: var SinglyLinkedList[T], value: T) {.inline.} =
assert a.contains(9)
prepend(L, newSinglyLinkedNode(value))


func copy*[T](a: SinglyLinkedList[T]): SinglyLinkedList[T] {.since: (1, 5, 1).} =
## Creates a shallow copy of `a`.
runnableExamples:
import sequtils
type Foo = ref object
x: int
var
f = Foo(x: 1)
a = [f].toSinglyLinkedList
let b = a.copy
a.add [f].toSinglyLinkedList
assert a.toSeq == [f, f]
assert b.toSeq == [f] # b isn't modified...
f.x = 42
assert a.head.value.x == 42
assert b.head.value.x == 42 # ... but the elements are not deep copied

let c = [1, 2, 3].toSinglyLinkedList
assert $c == $c.copy
result = initSinglyLinkedList[T]()
for x in a:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

result.append(x)

proc addMoved*[T](a, b: var SinglyLinkedList[T]) {.since: (1, 5, 1).} =
## Moves `b` to the end of `a`. Efficiency: O(1).
## Note that `b` becomes empty after the operation.
## Self-adding results in an empty list.
##
## See also:
## * `add proc <#add,T,T>`_
## for adding a copy of a list
runnableExamples:
import sequtils
var
a = [1, 2, 3].toSinglyLinkedList
b = [4, 5].toSinglyLinkedList
a.addMoved b
assert a.toSeq == [1, 2, 3, 4, 5]
assert b.toSeq == []
a.addMoved a
assert a.toSeq == []
if a.tail != nil:
a.tail.next = b.head
a.tail = b.tail
if a.head == nil:
a.head = b.head
b.head = nil
b.tail = nil

proc add*[T](L1: var SinglyLinkedList[T], L2: SinglyLinkedList[T]) {.since: (1, 5).} =
## Appends a shallow copy of `L2` to the end of `L1`.
runnableExamples:
var a = [1, 2, 3].toSinglyLinkedList
let b = [4, 5].toSinglyLinkedList
a.add b
assert a.toSeq == [1, 2, 3, 4, 5]
assert b.toSeq == [4, 5]
a.add a
assert a.toSeq == [1, 2, 3, 4, 5, 1, 2, 3, 4, 5]
var tmp = L2.copy
L1.addMove tmp

proc add*[T](L1: var SinglyLinkedList[T], L2: SinglyLinkedList[T]) {.since: (1, 5).} =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's not propagate pre-existing bad code :)
this violates style guide (caps)

instead use:

proc add*[T](a: var SinglyLinkedList[T], b: SinglyLinkedList[T]) {.since: (1, 5).} =

(or s/a/list/ but it's implied by the type)

future PR can fix the pre-existing style violations

## Appends a shallow copy of `L2` to the end of `L1`.
runnableExamples:
import sequtils
var a = [1, 2, 3].toSinglyLinkedList
let b = [4, 5].toSinglyLinkedList
a.add b
assert a.toSeq == [1, 2, 3, 4, 5]
assert b.toSeq == [4, 5]
a.add a
assert a.toSeq == [1, 2, 3, 4, 5, 1, 2, 3, 4, 5]
var tmp = L2.copy
L1.addMove tmp

proc append*[T](L: var DoublyLinkedList[T], n: DoublyLinkedNode[T]) =
## Appends (adds to the end) a node `n` to `L`. Efficiency: O(1).
Expand Down Expand Up @@ -523,6 +620,100 @@ proc prepend*[T](L: var DoublyLinkedList[T], value: T) =
assert a.contains(9)
prepend(L, newDoublyLinkedNode(value))

func copy*[T](a: DoublyLinkedList[T]): DoublyLinkedList[T] {.since: (1, 5, 1).} =
## Creates a shallow copy of `a`.
runnableExamples:
type Foo = ref object
x: int
var f = Foo(x: 1)
let
a = [f].toDoublyLinkedList
b = a.copy
f.x = 42
assert a.head.value.x == 42
assert b.head.value.x == 42

let c = [1, 2, 3].toDoublyLinkedList
assert $c == $c.copy
result = initDoublyLinkedList[T]()
for x in a:
result.append(x)

proc addMoved*[T](a, b: var DoublyLinkedList[T]) {.since: (1, 5, 1).} =
## Moves `b` to the end of `a`. Efficiency: O(1).
## Note that `b` becomes empty after the operation.
## Self-adding results in an empty list.
##
## See also:
## * `add proc <#add,T,T>`_
## for adding a copy of a list
runnableExamples:
import sequtils
var
a = [1, 2, 3].toDoublyLinkedList
b = [4, 5].toDoublyLinkedList
a.addMoved b
assert a.toSeq == [1, 2, 3, 4, 5]
assert b.toSeq == []
a.addMoved a
assert a.toSeq == []
if b.head != nil:
b.head.prev = a.tail
if a.tail != nil:
a.tail.next = b.head
a.tail = b.tail
if a.head == nil:
a.head = b.head
b.head = nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to:

  if a.addr != b.addr:
    b.head = nil
    b.tail = nil

and add this test:

  import lists, sequtils
  import std/enumerate
  import std/sugar
  var a = [0,1].toSinglyLinkedList # actualy, `toList` in your test
  a.addMoved a
  let s = collect:
    for i, ai in enumerate(a):
      if i >= 6: break
      ai
  doAssert s == [0,1,0,1,0,1]

and edit doc comment to:

  ## Note that `b` becomes empty after the operation.
  ## Self-adding results in an empty list.

=>

  ## Note that `b` becomes empty after the operation unless it has same address as `a`.

that's IMO a saner behavior

  • ditto with SinglyLinkedList

b.tail = nil

proc add*[T: SomeLinkedList](a: var T, b: T) {.since: (1, 5, 1).} =
## Appends a shallow copy of `b` to the end of `a`.
##
## See also:
## * `addMoved proc <#addMoved,SinglyLinkedList[T],SinglyLinkedList[T]>`_
## * `addMoved proc <#addMoved,DoublyLinkedList[T],DoublyLinkedList[T]>`_
## for moving the second list instead of copying
runnableExamples:
import sequtils
var a = [1, 2, 3].toSinglyLinkedList
let b = [4, 5].toSinglyLinkedList
a.add b
assert a.toSeq == [1, 2, 3, 4, 5]
assert b.toSeq == [4, 5]
a.add a
assert a.toSeq == [1, 2, 3, 4, 5, 1, 2, 3, 4, 5]
var tmp = b.copy
a.addMoved tmp

proc add*[T](L1: var DoublyLinkedList[T], L2: DoublyLinkedList[T]) {.since: (1, 5).} =
Copy link
Member

@timotheecour timotheecour Dec 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

factor (as other procs do in lists.nim):

proc add*[T: SomeLinkedList](a: var T, b: T) {.since: (1, 5).} =
  var c = b.copy
  a.addMove c

since its implementation is identical
(runnableExamples can show an example of each type, or not, (up to you); in any case both are tested in tlists.nim)

And a future prepend can also do like that

## Appends a shallow copy of `L2` to the end of `L1`.
runnableExamples:
import sequtils
var a = [1, 2, 3].toSinglyLinkedList
let b = [4, 5].toSinglyLinkedList
a.add b
assert a.toSeq == [1, 2, 3, 4, 5]
assert b.toSeq == [4, 5]
a.add a
assert a.toSeq == [1, 2, 3, 4, 5, 1, 2, 3, 4, 5]
var tmp = b.copy
a.addMoved tmp

proc add*[T](L1: var DoublyLinkedList[T], L2: DoublyLinkedList[T]) {.since: (1, 5).} =
## Appends a shallow copy of `L2` to the end of `L1`.
runnableExamples:
import sequtils
var a = [1, 2, 3].toSinglyLinkedList
let b = [4, 5].toSinglyLinkedList
a.add b
assert a.toSeq == [1, 2, 3, 4, 5]
assert b.toSeq == [4, 5]
a.add a
assert a.toSeq == [1, 2, 3, 4, 5, 1, 2, 3, 4, 5]
var tmp = b.copy
a.addMoved tmp

proc remove*[T](L: var DoublyLinkedList[T], n: DoublyLinkedNode[T]) =
## Removes a node `n` from `L`. Efficiency: O(1).
runnableExamples:
Expand Down
62 changes: 61 additions & 1 deletion tests/stdlib/tlists.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ discard """
targets: "c js"
"""

import lists
import lists, sequtils

const
data = [1, 2, 3, 4, 5, 6]
Expand Down Expand Up @@ -81,3 +81,63 @@ block tlistsToString:
l.append('2')
l.append('3')
doAssert $l == """['1', '2', '3']"""

template testCommon(initList, toList) =

block: # toSinglyLinkedList, toDoublyLinkedList
let l = seq[int].default
doAssert l.toList.toSeq == []
doAssert [1].toList.toSeq == [1]
doAssert [1, 2, 3].toList.toSeq == [1, 2, 3]

block copy:
Copy link
Member

@timotheecour timotheecour Dec 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test doesn't actually test correctness of copy (2 parts: source container is not modified; but elements are references)

(Remember that runnableExamples is only run for 1 specific backend, unlike this which will be tested on all backends (eventually, once #16384 is fixed))

EDIT: how about this:

  type Foo = ref object
    x: int
  let f0 = Foo(x: 0)
  let f1 = Foo(x: 1)
  var a = [f0]. toList
  var b = a.copy
  b.append f1
  doAssert a.toSeq == [f0]
  doAssert b.toSeq == [f0, f1]

doAssert array[0, int].default.toList.copy.toSeq == []
doAssert [1].toList.copy.toSeq == [1]
doAssert [1, 2].toList.copy.toSeq == [1, 2]
doAssert [1, 2, 3].toList.copy.toSeq == [1, 2, 3]
type Foo = ref object
x: int
var f = Foo(x: 1)
let a = [f, f].toList
f.x = 42
assert a.head.value == f
assert a.head.next.value == f

block: # add, addMoved
block:
var
l0 = initList[int]()
l1 = [1].toList
l2 = [2, 3].toList
l3 = [4, 5, 6].toList
l0.add l3
l1.add l3
l2.addMoved l3
doAssert l0.toSeq == [4, 5, 6]
doAssert l1.toSeq == [1, 4, 5, 6]
doAssert l2.toSeq == [2, 3, 4, 5, 6]
doAssert l3.toSeq == []
salvipeter marked this conversation as resolved.
Show resolved Hide resolved
l2.add l3 # re-adding l3 that was destroyed is now a no-op
doAssert l2.toSeq == [2, 3, 4, 5, 6]
doAssert l3.toSeq == []
l2.add(l3) # re-adding l3 that was destroyed is now a no-op
doAssert l2.toSeq == [2, 3, 4, 5, 6]
doAssert l3.toSeq == []
l2.add(l3) # re-adding l3 that was destroyed is now a no-op
doAssert l2.toSeq == [2, 3, 4, 5, 6]
doAssert l3.toSeq == []
block:
var
l0 = initList[int]()
l1 = [1].toList
l2 = [2, 3].toList
l3 = [4, 5, 6].toList
l3.addMoved l0
l2.addMoved l1
doAssert l3.toSeq == [4, 5, 6]
doAssert l2.toSeq == [2, 3, 1]
l3.add l0
doAssert l3.toSeq == [4, 5, 6]

testCommon initSinglyLinkedList, toSinglyLinkedList
testCommon initDoublyLinkedList, toDoublyLinkedList