Skip to content

Alternative implementation std::iter::Peekable::peek_mut #79419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

lukaslueg
Copy link
Contributor

This is an alternative implementation for std::iter::Peekable::peek_mut(), which has been added as an unstable feature peekable_peek_mut, tracked in #78302. Instead of returning an Option<&mut Item>, we return an &mut Option<Item>.

This allows us to control the iterator without advancing it. Namely, we can short-circuit the iterator (ending it) conditionally without advancing the underlying iterator during inspection. We can also rejuvenate an empty iterator after inspecting the next value. Both would not be possible without peeking, as there would be no way to place the new value back into the iterator if it doesn't fit the pattern. E.g.

// We must not advance the iterator in case it's next value is not actually `Some(&2)`.
if let p @ Some(&2) = iter.peek_mut() {
    *p = None;
}

// Without peeking, if `p` is `Some(T)`, there is no way to put it back
// Without `peek_mut`, we couldn't set the value.
if let p @ None = iter.peek_mut() {
    *p = Some(&10);
}

Both use-cases are already possible by simply calling peek() on the iterator and conditionally replacing the whole iterator (either the binding or through std::mem::replace) with a new iterator. This is cumbersome, though, as we need to know the exact type of the iterator in case it's unboxed (replacing with std::iter::empty or ::once won't do).

Since it previously wasn't possible to modify the state of Peekable from the outside, more tests may be needed to ensure that it behaves properly after it's peeked-member was modified?!

@rust-highfive
Copy link
Contributor

r? @withoutboats

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 25, 2020
@the8472
Copy link
Member

the8472 commented Nov 25, 2020

We can also rejuvenate an empty iterator after inspecting the next value.

That would violate requirements of some of the other traits implemented for Peekable such as ExactSizeIterator and InPlaceIterable

@lukaslueg
Copy link
Contributor Author

#79419 (comment) probably can't be solved through making this explicit in the docs. Feel free to close this if so desired.

@bors
Copy link
Collaborator

bors commented Nov 28, 2020

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@lukaslueg
Copy link
Contributor Author

closed due to concern in #79419 (comment)

@lukaslueg lukaslueg closed this Nov 28, 2020
@lukaslueg lukaslueg deleted the peek_mut_alt branch December 22, 2020 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants