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

make_contiguous before shrinking VecDeque #30

Merged
merged 1 commit into from
Feb 27, 2023
Merged

Conversation

trinity-1686a
Copy link
Contributor

No description provided.

@trinity-1686a trinity-1686a force-pushed the go-arround-std-ub branch 2 times, most recently from 44989f6 to 30e5ef6 Compare February 25, 2023 14:06
@trinity-1686a trinity-1686a marked this pull request as draft February 26, 2023 18:29
@trinity-1686a
Copy link
Contributor Author

we should at least check if it's worth to shrink before doing so, now that we have to use shrink_to_fit

@Sp00ph
Copy link

Sp00ph commented Feb 27, 2023

Just so you know, shrink_to_fit suffers from the same potential UB as shrink_to (e.g. running MIRI on this reports an uninitialized read: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=fed140485b86747eef2f61cfec434c63 ). I think that currently to safely shrink a VecDeque you have to call make_contiguous first. Alternatively, the fix (rust-lang/rust#108475 ) will probably be part of the next nightly, and also get backported into stable somewhat soon I believe.

@trinity-1686a
Copy link
Contributor Author

hm, okay. Thanks for the information!

@trinity-1686a trinity-1686a marked this pull request as ready for review February 27, 2023 09:01
@trinity-1686a trinity-1686a changed the title use shrink_to_fit while shrink_to can trigger UB make_contiguous before shrinking VecDeque Feb 27, 2023
@trinity-1686a trinity-1686a merged commit 1c864ed into main Feb 27, 2023
@trinity-1686a trinity-1686a deleted the go-arround-std-ub branch February 27, 2023 09:16
trinity-1686a added a commit that referenced this pull request Mar 10, 2023
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