Add vec::Drain{,Filter}::keep_rest

[WaffleLapkin]

This PR adds keep_rest methods to vec::Drain and vec::DrainFilter under drain_keep_rest feature gate:

// mod alloc::vec

impl<T, A: Allocator> Drain<'_, T, A> {
    pub fn keep_rest(self);
}

impl<T, F, A: Allocator> DrainFilter<'_, T, F, A>
where
    F: FnMut(&mut T) -> bool,
{
    pub fn keep_rest(self);
}

Both these methods cancel draining of elements that were not yet yielded from the iterators. While this needs more testing & documentation, I want at least start the discussion. This may be a potential way out of the "should DrainFilter exhaust itself on drop?" argument.

[rust-highfive]

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

[Gankra]

I haven't looked at the code, but I think this is a better answer to 90% of what drain_filter is trying to do, and the appropriate release valve for people who don't like drain's default semantic of "you have already drained all of the elements, now we shall yield them one at a time". It is a very "Entry" solution to the problem in that it provides an external interface to drain_filter's messy internal one... oh god is that terminology anyone uses anymore. crumbles to dust

[WaffleLapkin]

Truth be told, when I struggled with the default behavior of drain_filter I've implemented a crappy entry/cursor API for Vec. I guess Entry-like APIs are just superior answer to all problems.

[bors]

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

[JohnCSimon]

Ping from triage:
@WaffleLapkin can you please resolve the merge conflicts?

[the8472]

I think this should be the default for DrainFilter rather than an additional method.

Drain on the other hand already is stable so we can't change the default behavior and adding a method makes more sense there. But it's awkward ergonomically because those are iterators and you're presumably using them rather than retain() because you actually need the iterator part so you'd have to take it by_ref().

Perhaps adding a defaulted const generic Drain<T, const KEEP: bool = false> and keep_rest(self) -> Drain<T, true> would be more ergonomic.

[the8472]

r? rust-lang/libs-api

[bors]

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

[dtolnay]

I am on board with this API. Next steps are:

• I think this is the kind of method that would benefit from a usage example in the rustdoc.
• Needs a tracking issue.

Some considerations to note in the tracking issue for discussion before we could stabilize:

• Just change the not-yet-stable Drop for DrainFilter to keep rest?
  • Advantage: usually what you want (??)
    • I don't have a good picture what the use cases involve drain_filter and not exhausting the iterator obtained from it.
  • Disadvantage: inconsistent with stable Drop for Drain
  • If you want to remove rest (matching the current unstable behavior of drain_filter) then you'd need to write .foreach(drop) to explicitly drop all the rest of the range that matches the filter.
• &mut self instead of self?
  • If you're holding a Drain inside a struct and are operating on it from a &mut self method of the struct, keep_rest(self) is impossible to use. :(
    • You'd want something like mem::replace(&mut self.drain_filter, Vec::new().drain(..)).keep_rest() but the borrow checker won't like that.
    • Failing that, you'd need to change your Drain field to Option<Drain> and use self.drain_filter.take().unwrap().keep_rest() along with unwrap() everywhere else that the drain is used. Not good.
  • We'd need to define behavior of calling .next() after .keep_rest(). Presumably one of:
    • .next() returns None
    • this is considered incorrect usage and so .next() panicks
    • .keep_rest() sets a flag inside the Drain which Drop will use to decide its behavior, and you're free to continue accessing iterator elements in between.

[WaffleLapkin]

I've updated docs and the tracking issue ✅

[dtolnay]

Thanks!

[dtolnay]

@bors r+

[bors]

📌 Commit 7a433e4 has been approved by dtolnay

It is now in the queue for this repository.