add Vec::extend_from_within method under vec_extend_from_within feature gate

[WaffleLapkin]

Implement rust-lang/rfcs#2714

tl;dr

This PR adds a extend_from_within method to Vec which allows copying elements from a range to the end:

#![feature(vec_extend_from_within)]

let mut vec = vec![0, 1, 2, 3, 4];

vec.extend_from_within(2..);
assert_eq!(vec, [0, 1, 2, 3, 4, 2, 3, 4]);

vec.extend_from_within(..2);
assert_eq!(vec, [0, 1, 2, 3, 4, 2, 3, 4, 0, 1]);

vec.extend_from_within(4..8);
assert_eq!(vec, [0, 1, 2, 3, 4, 2, 3, 4, 0, 1, 4, 2, 3, 4]);

Implementation notes

Originally I've copied @Shnatsel's implementation with some minor changes to support other ranges:

pub fn append_from_within<R>(&mut self, src: R)
where
    T: Copy,
    R: RangeBounds<usize>,
{
    let len = self.len();
    let Range { start, end } = src.assert_len(len);;

    let count = end - start;
    self.reserve(count);
    unsafe {
        // This is safe because `reserve()` above succeeded,
        // so `self.len() + count` did not overflow usize
        ptr::copy_nonoverlapping(
            self.get_unchecked(src.start),
            self.as_mut_ptr().add(len),
            count,
        );
        self.set_len(len + count);
    }
}

But then I've realized that this duplicates most of the code from (private) Vec::append_elements, so I've used it instead.

Then I've applied @KodrAus suggestions from #79015 (comment).

[rust-highfive]

r? @KodrAus

(rust_highfive has picked a reviewer for you, use r? to override)

[leonardo-m]

I agree with Simon that this function feels a bit ad-hoc. But if you want to add this functionality then I think append_copy_from_within is a better name. For such uncommonly used function a long but clear name is OK (by Zipf's law).

[Shnatsel]

To be clear, the implementation is written by @WanzenBug, not myself. I have authored the RFC in question, but not the implementation.

The fact that the function feels niche is addressed in the RFC. TL;DR: this is a basic building block for many multimedia encoders/decoders, which they presently hand-roll using unsafe code and often get it wrong, leading to exploitable memory safety bugs.

[KodrAus]

It should already be possible to specialize this using our min_specialization subset, right? If that's the case we could simply make this method extend_from_within (to line up with Vec::extend_from_slice rather than with Vec::append, which moves elements instead of copying them) using a T: Clone bound, and then specialize the case where T: Copy?

If we don't want to try commit to specializing upfront then I'm ok with extend_from_within using a T: Copy bound for a start.

[Shnatsel]

If this can be reliably specialized for T: Copy, then I'm all for it, and naming it extend_from_within makes sense as you've described.

The T: Copy bound is required following the precedent set by slice::copy_from_slice and slice::copy_within, which guarantee that they desugar to a memcpy or memmove. However, Vec has no such functions, so not requiring T: Copy and specializing it behind the scenes will be the least surprising for API users. The question is, whether this can be done reliably.

Performance is crucial for this function: if it leaves performance on the table people will go back to hand-rolling this using ad-hoc unsafe code and getting it wrong.

[WaffleLapkin]

@KodrAus I don't know the details of the min_specialization, but as far, as I can tell, it is possible to use it here. At least my code compiles :D

I've also added a test which checks that the specialization was indeed used (so users may relay that it's cheap when T: Copy)

[Dylan-DPC-zz]

@KodrAus waiting for your review on this

[bors]

☔ The latest upstream changes (presumably #80530) made this pull request unmergeable. Please resolve the merge conflicts.

[rust-log-analyzer]

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

extracting /checkout/obj/build/cache/2020-11-19/rustfmt-nightly-x86_64-unknown-linux-gnu.tar.xz
error: failed to read `/checkout/src/tools/cargo/crates/credential/cargo-credential-1password/Cargo.toml`

Caused by:
  No such file or directory (os error 2)
Build completed unsuccessfully in 0:00:20

[jyn514]

@bors retry

Looks spurious. cc @rust-lang/infra, I think I've seen this error a couple times now.

extracting /checkout/obj/build/cache/2020-11-19/rustfmt-nightly-x86_64-unknown-linux-gnu.tar.xz
error: failed to read `/checkout/src/tools/cargo/crates/credential/cargo-credential-1password/Cargo.toml`

Caused by:
  No such file or directory (os error 2)
Build completed unsuccessfully in 0:00:20

[jyn514]

Oh wait, that was mingw-check that failed, sorry. Would still be great to find the cause though.

@bors r-

[rust-log-analyzer]

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

extracting /checkout/obj/build/cache/2020-11-19/rustfmt-nightly-x86_64-unknown-linux-gnu.tar.xz
error: failed to read `/checkout/src/tools/cargo/crates/credential/cargo-credential-1password/Cargo.toml`

Caused by:
  No such file or directory (os error 2)
Build completed unsuccessfully in 0:00:17

[WaffleLapkin]

I've tried to force-push to rerun CI, hoping that it's a phantom error, but since the error didn't change, I guess the error is stable.

[bors]

☔ The latest upstream changes (presumably #80758) made this pull request unmergeable. Please resolve the merge conflicts.

[rust-log-analyzer]

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

extracting /checkout/obj/build/cache/2020-11-19/rustfmt-nightly-x86_64-unknown-linux-gnu.tar.xz
error: failed to read `/checkout/src/tools/cargo/crates/credential/cargo-credential-1password/Cargo.toml`

Caused by:
  No such file or directory (os error 2)
Build completed unsuccessfully in 0:00:20

[bors]

☔ The latest upstream changes (presumably #80746) made this pull request unmergeable. Please resolve the merge conflicts.

[KodrAus]

Thanks @WaffleLapkin! This looks good to me.

@bors r+

[bors]

📌 Commit d5c2211 has been approved by KodrAus

[KodrAus]

@bors r-

Just noticed we need a tracking issue! I'll sort that now.

[KodrAus]

@bors r+

[bors]

📌 Commit 125ec78 has been approved by KodrAus

[bors]

⌛ Testing commit 125ec78 with merge f6cb45a...

[WaffleLapkin]

Yay. Thanks, @KodrAus, for reviewing!

[bors]

☀️ Test successful - checks-actions
Approved by: KodrAus
Pushing f6cb45a to master...