String/Vec::splice(RangeArgument, IntoIterator)

[SimonSapin]

Add a replace_slice method to Vec<T> and String removes a range of elements,
and replaces it in place with a given sequence of values.
The new sequence does not necessarily have the same length as the range it replaces.

Rendered.

[Stebalien]

Any reason you don't return an iterator that consumes the removed elements like splice? I'd also just call it splice for consistency.

Second, you can fall back on allocating temporary storage and copying twice if ExactSizeIterator returns the wrong length to avoid the quadratic time algorithm.

[SimonSapin]

Both good points.

[SimonSapin]

It looks like this functionality already exists! http://bluss.github.io/arrayvec/doc/odds/vec/trait.VecExt.html#tymethod.splice

[Gankra]

• I would like this functionality to just us vanilla size_hint, and do whatever dirty tricks it needs to do when the size_hint fails to match to make things work ok (allocating a back-up buffer seems fine).
• 👍 for splice; this is exactly that functionality but better.
• Do we specify that the old stream must be totally consumed first, and then the new stream is consumed (do we leave open the possibility for interleaving)?
• I agree that it should accept everything Extend does.

[dbrodie]

I think this is covered in #1253, just, as commented, should be generalized to Vec as well.

[SimonSapin]

I’ve updated the RFC to incorporate some feedback (rename to splice, return an iterator, fallback to allocating rather than taking quadratic time, …) and removed the implementation since I don’t have one yet that does all that. I can finish it once the exact semantics are decided.

@alexcrichton Could the libs team discuss this? Is it desired in std, and how should the unresolved questions be resolved?

[alexcrichton]

This all seems pretty reasonable to me, and the libs team process for this is to:

1. Assign a shepherd (@aturon in this case)
2. When the shepherd thinks that discussion has come to a conclusion, lull, etc, or the RFC is simply actionable, the libs team discussed moving into FCP
3. After a week of FCP, the libs team decides whether to merge or not based on discussion so far.

So along those lines anyone can always discuss this! If @aturon or anyone else on the libs team thinks this is ready for FCP then we'll likely move it in.

[dbrodie]

Another possibility is to add this as functionality to the Drain iterator. (This was @bluss's idea when I discussed it with him). Basically having something like: vec.drain(1..5).replace(other_iter). I personally find that this will be less discoverable there.

FWIW there are quite a few languages which have this operation as a first class feature, for example, Python (a[1:5] = b[3:5]).

[SimonSapin]

Indeed, now that this new method was changed to return an iterator I think it’s equivalent to Vec::drain when the input iterator is empty.

[tikue]

Would this be a duplicate fn if IndexAssign is eventually accepted? In the examples in that rfc they only assigned slices to vec ranges over the same len, but I don't see any reason it couldn't accommodate this usage as well.

[SimonSapin]

Would IndexAssign slicing work? Doesn’t IndexAssign need to take ownership of its parameter, which needs to be Sized?

[tikue]

I'm not sure which parameter you mean, but I was imagining this (I very well may be wrong here!):

Given:

trait IndexAssign<Index, Rhs> {
    /// `self[index] = rhs`
    fn index_assign(&mut self, index: Index, rhs: Rhs);
}

it would look like:

impl<R, I> IndexAssign<R, I> for Vec<T>
    where R: RangeArgument<usize>,
          I: IntoIterator<Item=T>
{
    // `vec[0..5] = (0..10);`
    fn index_assign(&mut self, index: R, rhs: I) {
        ...
    }
}

[tikue]

Ah, but I see splice is supposed to return the spliced-out slice. Perhaps that's just an oversight in the IndexAssign RFC.

[bluss]

The IndexAssign RFC /pull/1129 doesn't actually propose adding any operations to slices using IndexAssign.

[tikue]

True, but that could be revisited. It certainly seems like it's be nice.

[alexcrichton]

🔔 This RFC is now entering its week-long final comment period 🔔

[ollie27]

How about adding two methods splice() and splice_drain(), the first not returning anything and the second returning an iterator? It would mean if you don't actually want the existing elements (or there aren't any) you don't have to consume an iterator just to get the method to work.

[Stebalien]

@ollie27 In the current proposal, you don't have to consume the iterator, you just have to drop it.

[ollie27]

Well that's still an extra thing you have to do.

[tikue]

Drops are automatic. You don't have to explicitly do anything.
On Mar 3, 2016 4:43 PM, "Oliver Middleton" [email protected] wrote:

Well that's still an extra thing you have to do.

—
Reply to this email directly or view it on GitHub
#1432 (comment).

[ollie27]

Ah good point, I was thinking of assigning the result to a variable for some reason.

[bluss]

I think this should be Iterator else it won't work out. But that's a technicality.

[bluss]

The new flexible length semantics seem proportionally complex to me.

[SimonSapin]

@bluss I don’t understand your last comment. I guess flexible length refers to the iterator of new values not having to be the same length as the range it replaces. But what does proportionally complex mean?

[bluss]

splice found a nice way to solve it for any iterator I think. I just mean that splice is simple for good input (exact size-ish) and a bit more involved for the fallback cases, but it's not too complex. Complexity was my main worry with the expanded scope with non-exact size iterators.

[alexcrichton]

This RFC was discussed yesterday during libs triage, and the decision was to merge. We discussed whether we want APIs like this to run wild as it's unclear if the API of types like Vec will ever end, but on the other hand if we're gonna beef up types this seems like the type to beef up! Overall we concluded that this is pretty neat functionality to have now, so we're merging.

Thanks for the RFC @SimonSapin

[alexcrichton]

Tracking issue: rust-lang/rust#32310

[SimonSapin]

@alexcrichton Did the libs team discuss unresolved questions? In particular, when to consume the input iterator.

[aturon]

@SimonSapin We didn't discuss that in depth in the meeting (and in general are OK tweaking this point prior to stabilization).

From my perspective, I think it makes the most sense to consume on drop, given that there isn't necessarily a direct relationship between the number of items being drained and those being incorporated. (That is, if you had an interleaving semantics, in general interleaving would only happen for some prefix.) It seems simpler and more consistent to just do them all at once after the drop.

For the record, I'm just a little uncomfortable with so much of the action being tied to Drop here, just because it's subtle/clever. But I suspect over time the pattern will grow more comfortable/expected, so I'm happy to at least give it a shot (and it is a really neat trick!)

[SimonSapin]

I agree that doing so much in Drop is unusual, but there is some precedent in Vec::drain / Drain.