winnow-rs / winnow

Making parsing a breeze
https://docs.rs/winnow
Other
572 stars 44 forks source link

Cannot return iterators wrapping &mut ParserIterator #349

Open epage opened 1 year ago

epage commented 1 year ago

From rust-bakery/nom#1586 and fixed in rust-bakery/nom#1587

Prerequisites

Here are a few things you should provide to help me understand the issue:

* Rust version : `rustc 1.67.0-nightly (53e4b9dd7 2022-12-04)`

* nom version : `7.1.1`

* nom compilation features used: `[]` or `["std"]`

Test case

This code does not compile, but ideally should (play):

use nom::{
    character::complete::{digit1, newline},
    combinator::{iterator, opt},
    sequence::terminated,
    IResult,
};

fn parse_line(i: &str) -> IResult<&str, &str> {
    terminated(digit1, opt(newline))(i)
}

fn parse_input(i: &str) -> impl Iterator<Item = i32> + '_ {
    iterator(i, parse_line)
        // lots of processing, maybe a flat_map in between etc. - enough to warrant extracting it into a separate function
        .map(|x| x.parse::<i32>().unwrap())
}

fn main() {
    dbg!(parse_input("123\n456").collect::<Vec<_>>());
}

If I take a ParserIterator and use it as an Iterator, I can no longer return it from a function, since it's a Map<&mut ParserIterator>, and not a Map<ParserIterator>.

This is because the Iterator implementation of ParserIterator is on &mut ParserIterator and not ParserIterator directly. See here.

My guess is that this was done because one might later want to call .finish() on the parser iterator, and one can only do that if it hasn't been moved by a previous Iterator::map(self, Fn).

However, if instead the Iterator implementation was direcly on ParserIterator instead of &mut ParserIterator, the above example would compile. One could also decide not to move the iterator by calling .by_ref() on the ParserIterator, before applying .map().

Like this:

let it = iterator(i, parse_line);
it.by_ref()
  .map(|x| x.parse::<i32>().unwrap())
it.finish();

This would be as I understand it strictly more general, since it now allows for both returning a Map<ParserIterator>, and calling .finish() when needed (though not both at the same time), whereas the current variant excludes the possibility of returning a Map<ParserIterator>.

This proposal would however be a breaking change.

epage commented 11 months ago

For some discussion on this, see #392