Skip to content

Fix "called Option::unwrap() on a None value" in ParserIterator#1843

Open
forzafedor wants to merge 2 commits intorust-bakery:mainfrom
forzafedor:fix-unwrap-parser-iterator
Open

Fix "called Option::unwrap() on a None value" in ParserIterator#1843
forzafedor wants to merge 2 commits intorust-bakery:mainfrom
forzafedor:fix-unwrap-parser-iterator

Conversation

@forzafedor
Copy link
Copy Markdown

@forzafedor forzafedor commented Mar 24, 2025

Hello

In this example I had the following problem!

use std::iter::Iterator;

use nom::bytes::complete::tag;
use nom::character::complete::alphanumeric1;
use nom::combinator::iterator;
use nom::sequence::{separated_pair, terminated};
use nom::IResult;


fn main() {

  let data = "key1:value1,key2:value2,";
  let mut nom_it = iterator(
    data,
    terminated(
      separated_pair(alphanumeric1, tag(":"), alphanumeric1),
      tag(","),
    ),
  );

  assert_eq!(Some(("key1", "value1")), nom_it.next());
  assert_eq!(Some(("key2", "value2")), nom_it.next());
  assert_eq!(None, nom_it.next());
  assert_eq!(None, nom_it.next());
  let parser_result: IResult<_, _> = nom_it.finish();
  assert_eq!(Ok(("", ())), parser_result);
}
`thread 'main' panicked at /home/user/nom/src/combinator/mod.rs:912:29:
called `Option::unwrap()` on a `None` value`

This happened because of extra calls to the next() function and
Here the state variable is set to None but is not set in these conditions

I think this behavior is incorrect.
In iterator std::iter.next() additional calls to the next function will return None. I think it would be logical for this iterator to have similar behavior.

I fixed it and added tests for this example!

Please review MR you have time, thank you!

@forzafedor forzafedor requested a review from Geal as a code owner March 24, 2025 15:39
@asibahi
Copy link
Copy Markdown

asibahi commented Mar 30, 2025

Worth mentioning I removed the panic source in my (bigger scope) iterator PR: #1841

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants