Stabilize peekable_next_if

[Stupremee]

This PR stabilizes the peekable_next_if feature

Resolves #72480

[rust-highfive]

r? @withoutboats

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

[Stupremee]

r? @jyn514

[jyn514]

I'm not on T-libs, I shouldn't be the reviewer. r? @dtolnay maybe since they reviewed the initial PR, but it will need FCP either way.

[dtolnay]

@rust-lang/libs:
@rfcbot fcp merge

These two methods on std::iter::Peekable:

  impl<I: Iterator> Peekable<I> {
+     pub fn next_if(&mut self, func: impl FnOnce(&I::Item) -> bool) -> Option<I::Item>;

+     pub fn next_if_eq<T>(&mut self, expected: &T) -> Option<I::Item>
+     where
+         T: ?Sized,
+         I::Item: PartialEq<T>;
  }

let alnum = peekable.next_if(char::is_ascii_alphanumeric);

// equivalent to:
let alnum = match peekable.peek() {
    Some(ch) if ch.is_ascii_alphanumeric() => Some(peekable.next().unwrap()),
    _ => None,
};

https://doc.rust-lang.org/nightly/std/iter/struct.Peekable.html#method.next_if
https://doc.rust-lang.org/nightly/std/iter/struct.Peekable.html#method.next_if_eq

[rfcbot]

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @KodrAus
• @dtolnay
• @m-ou-se
• @sfackler
• @withoutboats

Concerns:

• mut resolved by Stabilize peekable_next_if #80011 (comment)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[m-ou-se]

@rfcbot concern mut

Now that Peekable::peek_mut also exists (#77491), I'm wondering if next_if should provide a &mut Item rather than a &Item. (To not make the same mistake as with Vec::retain, which in hindsight could've/should've given a &mut but only gives a &.) What do you all think?

[jyn514]

Why would you need to mutate the item?

[KodrAus]

I'm wondering if next_if should provide a &mut Item rather than a &Item

Oh that's an interesting philosophical question... Should we always try offer up the most exclusive reference we can?

Why would you need to mutate the item?

I wonder if we asked the same question about Vec::retain. The problem there is that in order to both retain and mutate you have to iterate the collection twice, isn't it?

With next_if it looks like there isn't much difference between next_if(filter).map(mutate) and a next_if_mut(filter_and_mutate_in_place). Since we stash the value in peeked, the semantics of next_if(|v| { mutate(v); false }) also seem a bit strange.

But I think is a shared reference sufficient when we could use an exclusive one? is a good question we should keep asking!

[KodrAus]

A next_if_mut method also doesn’t seem too offensive to me if we did want one. At least when reviewing code the _mut suffix would suggest there’s something more going on than just filtering and yielding values.

[KodrAus]

@m-ou-se Do you feel strongly that next_if should accept a &mut item, or are you satisfied with some combination of .peek_mut(mutate).next_if(condition_after_mutate) or .peek().next_if(condition).map(mutate), with a possible next_if_mut alternative introduced later if it proves useful?

[m-ou-se]

That sounds good to me.

@rfcbot resolve mut

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

[dtolnay]

@bors r+

[bors]

📌 Commit ceda547 has been approved by dtolnay

[m-ou-se]

Note: If #81810 fails but #81792 (which is in the queue right after) gets merged, this will need to be updated to since = "1.52.0" instead.