Description
Proposal
Problem statement
Similar to Iterator::filter_map, sometimes it is more natural to combine Peekable::next_if and Option::map into a single operation.
Motivating examples or use cases
Consider the following code to unescape octal-escape sequences:
fn unescape_octal(a: &str) -> String {
let mut it = Peekable::new(a.chars());
let mut res = String::with_capacity(a.len());
while let Some(c) = it.next() {
if c == '\\' {
let mut codepoint = 0;
/*------------------------------------------------------*/
/* read the following [0-7]* and parse into `codepoint` */
/*------------------------------------------------------*/
res.push(char::from_u32(codepoint).unwrap());
} else {
res.push(c);
}
}
res
}
assert_eq!(unescape_octal(r"ab\377\570c\144e\146"), "abÿŸcdef");There are currently 2 ways to implement /* read the following [0-7]* and parse into `codepoint` */, each with some small disadvantages:
-
Using
.next_if(), but it requires evaluating the condition twice and there is an unnecessary.unwrap():while let Some(o) = it.next_if(|c| c.is_digit(8)) { codepoint = codepoint * 8 + o.to_digit(8).unwrap(); }
-
Using
.peek(), but the user has to remember to call.next()while let Some(o) = it.peek() && let Some(digit) = o.to_digit(8) { it.next(); codepoint = codepoint * 8 + digit; }
Solution sketch
The .next_if()'s condition takes an impl FnOnce(&I::Item) -> bool as input, and returns the item on succeed.
We can create a variant which takes an impl FnOnce(&I::Item) -> Option<R> as input, with Some(r) representing "success" (which the item is consumed) and None "failure".
impl<I: Iterator> Peekable<I> {
pub fn map_next_if_some<R>(&mut self, func: impl FnOnce(&I::Item) -> Option<R>) -> Option<R> {
let item = self.peek()?;
let result = func(item)?;
self.peeked = None; // equivalent to `self.next()`
Some(result)
}
}The motivating example can then be implemented as:
while let Some(digit) = it.map_next_if_some(|c| c.to_digit(8)) {
codepoint = codepoint * 8 + digit;
}Alternatives
-
I am not too fond of the name
map_next_if_*. I expect better name suggestions. -
One may also provide an alternative interface which takes an owned value:
pub fn map_next_if_ok<R>(&mut self, func: impl FnOnce(I::Item) -> Result<R, I::Item>) -> Option<R> { let unpeek = if let Some(item) = self.next() { match func(item) { Ok(result) => return Some(result), Err(item2) => Some(item2), } } else { None }; self.peeked = Some(unpeek); None } // usage: while let Some(digit) = it.map_next_if_ok(|c| c.to_digit(8).ok_or(c)) { codepoint = codepoint * 8 + digit; }
This allows us to provide an alternative solution to Add
iter::Peekable::next_unpeek#557:while let Some(s) = parser.map_next_if_ok(|event| match event { pulldown_cmark::Event::Text(s) => Ok(s), other => Err(other), }) { do_something(s); }
This is not saying this ACP is supposed to entirely replace
.peek_slot()though. For instance themap_next_ifapproach cannot scale to multiple match arms doing different things like this:loop { let peek_slot = parser.peek_slot(); match peek_slot.take() { Some(pulldown_cmark::Event::Text(text)) => do_something(text), Some(pulldown_cmark::Event::Code(code)) => do_something_else(code), // <-- new branch can be easily added other => { *peek_slot = other; // <-- still, forgetting to unpeek will be a foot gun. break; } } }
Similar to
.peek_slot()the user may unpeek an entirely irrelevant item withErr(custom_value). This is already possible with.peek_mut()(stable since 1.53) so it is not considered a problem.
Links and related work
Some real world example code which can be improved by this ACP (by eliminating some unreachable!() branches or redundant checks)
while let Some(next_expected_idx) = errors.map_next_if_some(|e| match e {
Error::Missing(&next_expected_idx)
if next_expected_idx == *missing_idxs.last().unwrap() + 1 =>
{
Some(next_expected_idx)
}
_ => None,
}) {
missing_idxs.push(expected_idx);
} while let Some(t) = self.parser.map_next_if_ok(|event| match event {
Event::Text(t) => Ok(t),
Event::SoftBreak => Ok("\n".into()),
other => Err(other),
}) {
text.push_str(&t);
}fn map_yoon_char(ch: &char) -> Option<char> {
match *ch {
'a' => Some('ゃ'),
'u' => Some('ゅ'),
'o' => Some('ょ'),
_ => None,
}
}
...
if let Some(yoon) = parser.map_next_if_some(map_yoon_char) {
output.push('に');
output.push(yoon);
} else {
output.push(ch);
} let transactions = core::iter::from_fn(|| {
spi_operations.map_next_if_ok(|operation| match operation {
SpiOperation::Transaction(transaction) => Ok(transaction),
other => Err(other),
})
});https://github.com/parasyte/myn/blob/ba6980a9edd3c13cca6e1d643fae8c43fea730fd/src/ty.rs#L193-L196
self.map_next_if_ok(|token| match token {
TokenTree::Group(group) => Ok(group),
other => Err(other),
})
.ok_or_else(|| spanned_error("Expected group", Span::call_site())) let source_info = lines.map_next_if_ok(|line| match line {
ParsedLine::BacktraceSource(source_info) => Ok(source_info),
other => Err(other),
});What happens now?
This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.
Possible responses
The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):
- We think this problem seems worth solving, and the standard library might be the right place to solve it.
- We think that this probably doesn't belong in the standard library.
Second, if there's a concrete solution:
- We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
- We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.