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

DList update #7944

Closed
wants to merge 10 commits into from
Closed

DList update #7944

wants to merge 10 commits into from

Conversation

bluss
Copy link
Member

@bluss bluss commented Jul 21, 2013

Factor out internal methods to pop/push list nodes so that .merge() and .rotate_to_front(), .rotate_to_back() (new methods) can be implemented without allocating nodes.

With that, some cleanup changes to DList use of Option, and adding a missing Encodable implementation.

blake2-ppc added 9 commits July 21, 2013 19:31
This impl was missing for unknown reason.
Make the core Deque implementation a bit simpler by using Option methods
when we simply map on a Some value, and deduplicate some common lines.
Factor out internal methods for pop/push ~Node<T>, This allows moving
nodes instead of destructuring and allocating new.

Make use of this in .merge() so that it requires no allocations when
merging two DList.
Add methods to move back element to front or front element to back,
without reallocating nodes.
These tests for ~[] performance don't really belong here, they were for
comparison.
We don't need TotalOrd for ordered insertion, just the normal sort order
Ord.
.peek_next() needs to check the element counter just like the .next()
and .next_back() iterators do.

Also clarify .insert_next() doc w.r.t double ended iteration.
@bluss
Copy link
Member Author

bluss commented Jul 22, 2013

The commit dlist: Add bench test for rotate_to_{front, back} bfa9b43 and those following it were added after the PR was opened, they are all just smaller cleanups and a bug fix (peek_next()) that might as well go in here.

@bblum
Copy link
Contributor

bblum commented Jul 22, 2013

Otherwise looks good, feel free to r=bblum once you decide how to address my above comment.

@bblum
Copy link
Contributor

bblum commented Jul 22, 2013

I didn't realize DList had turned into an owned data structure with raw back pointers in the time since I wrote it. It's good to see.

@bluss
Copy link
Member Author

bluss commented Jul 22, 2013

I appreciate the comment on naming. At the moment nodes are not exposed at all, so methods taking nodes are not possible.

bors added a commit that referenced this pull request Jul 23, 2013
Factor out internal methods to pop/push list nodes so that .merge() and .rotate_to_front(), .rotate_to_back() (new methods) can be implemented without allocating nodes.

With that, some cleanup changes to DList use of Option, and adding a missing Encodable implementation.
@bors bors closed this Jul 23, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 23, 2021
…=giraffate

Add MSRV to deprecated_cfg_attr

changelog: Add MSRV to [`deprecated_cfg_attr`]

closes: rust-lang#7922
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants