fixed VecDeque::splice() not filling the buffer correctly when resizing the buffer on start = end range#152258
Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom Mar 13, 2026
Conversation
Collaborator
|
|
This comment has been minimized.
This comment has been minimized.
d33db94 to
2ecddd2
Compare
joboet
requested changes
Feb 7, 2026
Collaborator
|
Reminder, once the PR becomes ready for a review, use |
asder8215
commented
Feb 7, 2026
65cec58 to
748d3c1
Compare
Contributor
Author
|
@rustbot ready |
joboet
requested changes
Feb 12, 2026
joboet
requested changes
Feb 21, 2026
440a328 to
409bb2f
Compare
Member
|
Looks great, thank you! Could you squash your commits, please? I'll approve after that. |
409bb2f to
bb67ee2
Compare
Contributor
Author
|
@joboet put the change into one commit. Lmk if there's anything else I need to do! |
joboet
requested changes
Feb 28, 2026
…ng the buffer on start = end range
bb67ee2 to
d0a33ab
Compare
Contributor
Author
|
Hope the new comment/rationale on the change is okay! |
Contributor
Author
|
@joboet I apologize for the ping, was wondering if it would be possible for you to review this within a couple of days? |
Member
|
@bors r+ |
Contributor
Zalathar
added a commit
to Zalathar/rust
that referenced
this pull request
Mar 13, 2026
…r=joboet fixed VecDeque::splice() not filling the buffer correctly when resizing the buffer on start = end range This PR fixes rust-lang#151758. The issue came from `Splice::move_tail`, which as joboet pointed out: > The issue is in move_tail, which resizes the buffer, but fails to account for the resulting hole. The problem with reserving more space through `VecDeque`'s `buf.reserve()` is that it doesn't update `VecDeque`'s `head`, which means that this code in `move_tail`: ```rust deque.wrap_copy( deque.to_physical_idx(tail_start), deque.to_physical_idx(new_tail_start), self.tail_len, ); ``` could move over the section of data that `tail_start..tail_start + self.tail_len` of the buffer is supposed to be held at to the incorrect destination since all `.to_physical_idx()` is doing is a wrapping add on the `VecDeque`'s head with the passed in `idx` value. To avoid this I decided to use `VecDeque::reserve` to both allocate more space into the `VecDeque` if necessary and update head appropriately. However, `VecDeque::reserve` internally relies on the `VecDeque`'s `len` field. Earlier in `VecDeque::splice`, it modifies the `VecDeque`'s `len` constructing the drain via `Drain::new` (as it does a `mem::replace` on `deque.len` with the start bound of the passed in `range`). The `VecDeque`'s `len` can also be potentially modified in the earlier `Splice::fill()` call if it does any replacement towards elements within the passed in `range` value for `VecDeque::splice()`. I needed to temporarily restore the `VecDeque`'s `len` to its actual len, so that `VecDeque::reserve` can work properly. Afterward, you can bring back the `VecDeque`'s `len` to what its value was before and fill the gap appropriately with the rest of the `replace_with` content. r? @joboet
This was referenced Mar 13, 2026
rust-bors bot
pushed a commit
that referenced
this pull request
Mar 13, 2026
Rollup of 5 pull requests Successful merges: - #152258 (fixed VecDeque::splice() not filling the buffer correctly when resizing the buffer on start = end range) - #153691 (Miscellaneous tweaks) - #153766 (`try_execute_query` tweaks) - #153188 (implementation of `-Z min-recursion-limit`) - #153428 (Avoid duplicated query modifier comments.)
github-actions bot
pushed a commit
to rust-lang/rustc-dev-guide
that referenced
this pull request
Mar 16, 2026
Rollup of 5 pull requests Successful merges: - rust-lang/rust#152258 (fixed VecDeque::splice() not filling the buffer correctly when resizing the buffer on start = end range) - rust-lang/rust#153691 (Miscellaneous tweaks) - rust-lang/rust#153766 (`try_execute_query` tweaks) - rust-lang/rust#153188 (implementation of `-Z min-recursion-limit`) - rust-lang/rust#153428 (Avoid duplicated query modifier comments.)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes #151758. The issue came from
Splice::move_tail, which as joboet pointed out:The problem with reserving more space through
VecDeque'sbuf.reserve()is that it doesn't updateVecDeque'shead, which means that this code inmove_tail:could move over the section of data that
tail_start..tail_start + self.tail_lenof the buffer is supposed to be held at to the incorrect destination since all.to_physical_idx()is doing is a wrapping add on theVecDeque's head with the passed inidxvalue.To avoid this I decided to use
VecDeque::reserveto both allocate more space into theVecDequeif necessary and update head appropriately. However,VecDeque::reserveinternally relies on theVecDeque'slenfield. Earlier inVecDeque::splice, it modifies theVecDeque'slenconstructing the drain viaDrain::new(as it does amem::replaceondeque.lenwith the start bound of the passed inrange). TheVecDeque'slencan also be potentially modified in the earlierSplice::fill()call if it does any replacement towards elements within the passed inrangevalue forVecDeque::splice(). I needed to temporarily restore theVecDeque'slento its actual len, so thatVecDeque::reservecan work properly. Afterward, you can bring back theVecDeque'slento what its value was before and fill the gap appropriately with the rest of thereplace_withcontent.r? @joboet