add VecDeque::splice #147247
add VecDeque::splice #147247
Conversation
rustbot
added
S-waiting-on-review
|
Thanks for working on this! |
|
Sorry I haven't had a chance to look at this, still behind on my reviews r? libs |
|
☔ The latest upstream changes (presumably #148337) made this pull request unmergeable. Please resolve the merge conflicts. |
| // This will set drain.remaining to 0, so its drop won't try to read deallocated memory on | ||
| // drop. | ||
| self.drain.by_ref().for_each(drop); | ||
|
|
There was a problem hiding this comment.
Do we need something like
rust/library/alloc/src/vec/splice.rs
Line 62 in 87f9dcd
There was a problem hiding this comment.
As I understand it, the for_each will ensure that remaining is set to 0, so Drain::drop won't drop anything. I intentionally use Drain::drop to put the elements together again properly.
| /// This is optimal if: | ||
| /// | ||
| /// * The tail (elements in the deque after `range`) is empty, | ||
| /// * or `replace_with` yields fewer or equal elements than `range`'s length | ||
| /// * or the lower bound of its `size_hint()` is exact. | ||
| /// | ||
| /// Otherwise, a temporary vector is allocated and the tail is moved twice. |
There was a problem hiding this comment.
Can we add a note to the tracking issue to confirm this is accurate? I'm not sure but it feels like there ought to be slightly different conditions vs. Vec, since we have a potentially 'empty' middle to play with...
There was a problem hiding this comment.
I guess this is what the "Unresolved Questions" section is for? This is my first tracking issue
added it there
| unsafe fn move_tail(&mut self, additional: usize) { | ||
| let deque = unsafe { self.deque.as_mut() }; | ||
| let tail_start = deque.len + self.drain_len; | ||
| deque.buf.reserve(tail_start + self.tail_len, additional); |
There was a problem hiding this comment.
Do we have coverage for this reserve running? Normally growing VecDeque requires extra logic AFAICT, not just reserving the underlying buffer.
There was a problem hiding this comment.
It should run anytime the new iterator (replace_with) has a different length than the removed elements, which is definitely tested. Do you have a more specific case in which this could cause issues?
rustbot
added
S-waiting-on-author
Qelxiros
force-pushed
the
vecdeque_splice
branch
from
a9f20e3 to
003ebda
Compare
This comment has been minimized.
This comment has been minimized.
Qelxiros
force-pushed
the
vecdeque_splice
branch
from
003ebda to
f01287c
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
rustbot
added
S-waiting-on-review
|
I'm not sure why the CI isn't running, but all the requested changes should be good |
6 participants
Tracking issue: #146975