Make Vec::clone_from and slice::clone_into share the same code #107424
Conversation
This file contains 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
The implementation for the ToOwned::clone_into method on [T] is a copy of the code for vec::clone_from. In 3613980 the code for vec::clone_from gained a specialization for when T is Copy. This commit copies that specialization over to the clone_into implementation.
In the past, Vec::clone_from was implemented using slice::clone_into. The code from clone_into was later duplicated into clone_from in 8725e4c, which is the commit that adds custom allocator support to Vec. Presumably this was done because the slice::clone_into only works for vecs with the default allocator so it would have the wrong type to clone into Vec<T, A>. Now that the clone_into implementation is moved out into a specializable trait anyway we might as well use that to share the code between the two methods.
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon. Please see the contribution instructions for more information. |
This comment was marked as resolved.
This comment was marked as resolved.
scottmcm
approved these changes
Jan 30, 2023
|
@bors r+ Thanks for spotting and fixing this! (And sorry for totally missing your note in the PR description about the custom allocators.) |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 30, 2023
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#107125 (Add and use expect methods to hir.) - rust-lang#107172 (Reimplement NormalizeArrayLen based on SsaLocals) - rust-lang#107177 (Keep all theme-updating logic together) - rust-lang#107424 (Make Vec::clone_from and slice::clone_into share the same code) - rust-lang#107455 (use a more descriptive name) - rust-lang#107465 (`has_allow_dead_code_or_lang_attr` micro refactor) - rust-lang#107469 (Change turbofish context link to an archive link) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
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.
In the past,
Vec::clone_fromwas implemented usingslice::clone_into. The code fromclone_intowas later duplicated intoclone_fromin 8725e4c, which is the commit that adds custom allocator support to Vec. Presumably this was done because theslice::clone_intomethod only works for vecs with the default allocator so it would have the wrong type to clone intoVec<T, A>.Later on in 3613980 the code for the two methods diverged because the
Vec::clone_fromversion gained a specialization to optimize the case when T is Copy. In order to reduce code duplication and make them both be able to take advantage of this specialization, this PR moves the specialization into the slice module and makes vec use it again.