Tracking issue for copy_within

[oconnor663]

Rust PR: #53652
Standalone implementation: https://github.com/oconnor663/copy_in_place

Summary: This is a safe wrapper around ptr::copy, for regions within a single slice. Previously, safe in-place copying was only available as a side effect of Vec::drain.

Open questions: @SimonSapin had a proposal for an alternative API using ranges. Here's an example PR along those lines: oconnor663/copy_in_place#1

• implement as a PR (define copy_within on slices #53652)
• land as unstable
• decide on a final API
• stabilize

[oconnor663]

@alexcrichton what do the next steps look like for something like this? How do we "decide" on an API? :)

[alexcrichton]

@oconnor663 the process here is somewhat informal, but after the API has been in nightly for some time it's renominated for stabilization with any open questions, and at that point the libs team all signs off on the stabilization.

To that end it's basically "let this sit for awhile and gain feedback", and then after awhile has passed it can be proposed to be stabilized

[briansmith]

FWIW, I tried this out in ring and here's my feedback:

• I wish there was a non-panicking version that failed with an error result when the bounds are out of range. I ended up having to duplicate the range checking in my own code.

• It is very important that the implementation be inlined but the implementation doesn't have #[inline]. In practice the compiler was always inlining it for me but it would be good to do something more to ensure this.

[oconnor663]

Thanks for trying it out.

1. What's the situation where you need to check against making an out of bounds copy? Would it be possible to see the code where this came up? What fallback would you be using if copy_within returned an error?

2. make copy_within #[inline] #56886

[briansmith]

What's the situation where you need to check against making an out of bounds copy? Would it be possible to see the code where this came up? What fallback would you be using if copy_within returned an error?

I have my own API that is like copy_within, but which does an encryption/decryption operation as part of the copying, but my API avoids panicking on invalid bounds. There wouldn't be any fallback used; I just return an error.

[oconnor663]

I don't imagine it would make sense to hoist the bounds checking requirement up to the caller, and just have ring propagate the panic if the bounds are wrong? I guess the question is whether any of the callers are expected to have a fallback, or whether bad bounds are more likely to be programmer error.

[briansmith]

I don't imagine it would make sense to hoist the bounds checking requirement up to the caller, and just have ring propagate the panic if the bounds are wrong? I guess the question is whether any of the callers are expected to have a fallback, or whether bad bounds are more likely to be programmer error.

There are lots of possibilities. This isn't the only API in libstd that has only a panicking version so if that's what libstd people like, I would just carry on as-is.

[oconnor663]

Yeah I was definitely working by analogy with copy_from_slice and friends.

[Shnatsel]

FWIW this is desired by Ropey crate. Currently unsafe code is used to achieve this.

[SimonSapin]

@briansmith Adding #[inline] sounds good, do you want to send a PR?

The arguments for a non-panicking version also sound good to me. Perhaps even instead of, not in addition to what’s implemented today. And users that want panic can call unwrap.

What API should this have? Perhaps a Result return type rather than Option to take advantage of #[must_use], but there is no precedent in std for Result<…, ()>, so with a new dedicated error type?

The other API that does non-panicky bounds checking of a range argument is [T]::get which returns an Option, but that one doesn’t have side effects and is only useful for its return value.

[oconnor663]

@SimonSapin I put an #[inline] change up as #56886, but @sfackler was opposed to it. There's some discussion in the PR.

I feel like I remember some General Guidance from many years ago, when there was a big cleanup going on in the stdlib, about when to have panicking vs option/result APIs or when to have both. Does anyone remember where that was or what the consensus was there?

[oconnor663]

@Shnatsel this function is available in the tiny copy_in_place crate, if you want to keep the unsafe code out of the caller.

[joshlf]

To that end it's basically "let this sit for awhile and gain feedback", and then after awhile has passed it can be proposed to be stabilized

Has it been a while yet? :) I'd like to use this if stabilized.

[kennytm]

Filed a stabilization PR at #61398.