Skip to content
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

Cannot return iterators wrapping &mut ParserIterator #349

Open
epage opened this issue Oct 23, 2023 · 1 comment
Open

Cannot return iterators wrapping &mut ParserIterator #349

epage opened this issue Oct 23, 2023 · 1 comment
Labels
A-combinator Area: combinators C-enhancement Category: Raise on the bar on expectations M-breaking-change Meta: Implementing or merging this will introduce a breaking change.
Milestone

Comments

@epage
Copy link
Collaborator

epage commented Oct 23, 2023

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 epage added A-combinator Area: combinators M-breaking-change Meta: Implementing or merging this will introduce a breaking change. C-enhancement Category: Raise on the bar on expectations labels Oct 23, 2023
@epage
Copy link
Collaborator Author

epage commented Dec 18, 2023

For some discussion on this, see #392

@epage epage added this to the 0.7.0 milestone Jan 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-combinator Area: combinators C-enhancement Category: Raise on the bar on expectations M-breaking-change Meta: Implementing or merging this will introduce a breaking change.
Projects
None yet
Development

No branches or pull requests

1 participant