buggy (or unexpected) interaction between `iterator` and `parse_complete`

[asibahi]

Hello,

I ran into an unexpected panic. I managed to reduce into this code:

use nom::{
    Finish, IResult, Parser,
    bytes::tag,
    combinator::{all_consuming, iterator},
};

fn main() {
    let data = "hellohellohellohel";
    let parser = all_consuming(parse_list).parse_complete(data).finish();
}

fn parse_list(i: &str) -> IResult<&str, Vec<&str>, ()> {
    let mut iter = iterator(i, tag("hello"));

    let result = iter.by_ref().collect();

    let (i, ()) = iter.finish()?;

    Ok((i, result))
}

error message:

Cannot call `finish()` on `Err(Err::Incomplete(_))`: this result means that the parser does not have enough data to decide, you should gather more data and try to reapply the parser instead

Considering neither all_consumingnor parse_complete should return Incomplete, this was a bit of a surprise. I am not sure where the mishap is.

In my actual code I managed to avoid it by doing this.

    let (i, ()) = iter.finish().map_err(|e| match e {
        nom::Err::Incomplete(_) => {
            nom::Err::Error(error::make_error(i, error::ErrorKind::TooLarge))
        }
        fail => fail,
    })?;

[shahn]

What happens is that the OutputMode (which you set to Complete by calling parse_complete() is not passed all the way to tag("hello"). This happens because inside ParseIterator's Iterator impl, anything you pass into it is implicitly used as a streaming parser (the bug is the .parse(input) instead of .parse_complete(input) when in Complete mode):

impl<Input, Output, Error, F> core::iter::Iterator for ParserIterator<Input, Error, F>
where
  F: Parser<Input, Output = Output, Error = Error>,
  Input: Clone,
{
  type Item = Output;

  fn next(&mut self) -> Option<Self::Item> {
    if let State::Running = self.state.take().unwrap() {
      let input = self.input.clone();

      match (self.iterator).parse(input) {     <--- Problem here
        Ok((i, o)) => {
          self.input = i;
          self.state = Some(State::Running);
          Some(o)
        }
        Err(Err::Error(_)) => {
          self.state = Some(State::Done);
          None
        }
        Err(Err::Failure(e)) => {
          self.state = Some(State::Failure(e));
          None
        }
        Err(Err::Incomplete(i)) => {
          self.state = Some(State::Incomplete(i));
          None
        }
      }
    } else {
      None
    }
  }
}

You could fix this by using nom::bytes::complete::tag instead of nom::bytes::tag inside your parse_list fn, but the behaviour of nom here is still a bug (at least in the documentation, but overall it seems to go against the idea of nom v8).

I am not sure how to deal with this nicely given the large amount of functions inside nom which still return IResult. It would help to get some guidance on the next steps for nom :)

[shahn]

I guess one argument might be that iterator() generally doesn't make that much sense for streaming parsers, as there is no way to provide more data without iterating over the first successfully parsed elements. Hrm...

[asibahi]

I have come up with a potential solution by adding a generic IsStreaming argument to iterator and all associated data structures, in addition to a separate finish_complete method. I do not particularly like it (and it is also a breaking change) which is why I have not made a PR. However, I would appreciate a review.

https://github.com/asibahi/nom/tree/iterator

Edit 2: kept the old iterator function the same but added another iterator_complete. This change is backwards compatible, so I made a PR.