[stdlib] Enhance Linked List #3942
Conversation
owenhilyard
force-pushed
the
linkedlist
branch
from
5b54853 to
557b07c
Compare
| # c = c[].next | ||
| return current | ||
|
|
||
| fn __getitem__[I: Indexer](ref self, read idx: I) raises -> ref [self] T: |
There was a problem hiding this comment.
I thought it was decided a while ago to just use Int here and let the implicit conversion do the work?
There was a problem hiding this comment.
I was following what List does. If we want to establish that as a convention, I think that it probably should go in the stdlib docs.
There was a problem hiding this comment.
Ah I see now it was recently changed back to this way. That's fine I preferred it anyways
| Returns: | ||
| Whether the list is empty or not. | ||
| """ | ||
| return self._head == Self.PointerT() |
There was a problem hiding this comment.
Why not use return not self._head instead?
There was a problem hiding this comment.
I had "check head for null" in my head when writing this. I'll remove the comparison.
owenhilyard
force-pushed
the
linkedlist
branch
2 times, most recently
from
9743f28 to
e46ebca
Compare
|
Move to draft due to existing linked list. I'll circle around after I've merged the two implementations. |
That sounds super weird. I'm not sure what is going on, but if you could reduce this down to a small testcase, I would be happy to take a look at some point. |
Merges my earlier work with the one which appeared in the stdlib, expanding the API and test coverage. Also makes a few of the test_utils counters `Writable` to help them conform to more APIs. Signed-off-by: Owen Hilyard <[email protected]>
owenhilyard
force-pushed
the
linkedlist
branch
from
e46ebca to
205ee8d
Compare
|
@lattner not sure if you want one without the attached data structure, but if I replace stdlib/test/collections/linked_list.mojo:464 with |
|
cc @abduld as the original author. |
There was a problem hiding this comment.
Just did a quick pass of this, looking good! Left some comments and @lsh will also take a look.
Thanks for working on improving this a ton from the basic implementation that was started prior to your work! Sorry that we already had a PR in-flight starting the linked list type.
That's fine, I think this shows a need for external stdlib contributors to be able to do some kind of work tracking and for there to be some visibility into non-sensitive work done internally before it hits nightly. Even on the level of a discord channel to toss "I'm working on a linked list" messages into. |
Minor tweaks, mostly consisting of removing unnecessary things like some leftover debug prints and explicit type parameters. Also adds time complexity information to all public methods of `LinkedList`. Signed-off-by: Owen Hilyard <[email protected]>
|
I'm not going to have time to finish up the last few bits of work and deal with the merge conflict today, and potentially not tomorrow. |
Adds a
LinkedListtype which implements most of the API surface ofList(also grabbing the tests). This should help with filling in basic data structures in the stdlib.No iterator because I ran into some nasty issues with origins and I'm not sure how far I can push casting them before I get to UB.
There's also an instance of
list.__getitem__(0).__getitem__(1).value = "Mojo"in test_linked_list.mojo since I couldn't find another way to avoid the copy. If someone has a solution please let me know.