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

Empty choice panics when trying to parse. #668

Open
DaniD3v opened this issue Sep 6, 2024 · 4 comments
Open

Empty choice panics when trying to parse. #668

DaniD3v opened this issue Sep 6, 2024 · 4 comments

Comments

@DaniD3v
Copy link

DaniD3v commented Sep 6, 2024

struct ParsableType;

fn parser() -> impl Parser<char, ParsableType, Error = ParserError> {
    let parsers: [BoxedParser<'_, char, ParsableType, Simple<char>>; 0] = [];
    choice(parsers)
}

#[test]
fn this_panics() {
    let _ = parser().parse("");
}
---- parser::bin_ops::this_panics stdout ----
thread 'parser::bin_ops::this_panics' panicked at /home/notyou/.cargo/registry/src/index.crates.io-6f17d22bba15001f/chumsky-0.9.3/src/primitive.rs:945:30:
called `Option::unwrap()` on a `None` value

Having an empty choice is reasonable and useful.
Consider having a Vec<Option<impl Parser>> and then using .into_iter().flatten().

If you don't agree there should at least be an error message telling the user what went wrong.

here's the full backtrace:

---- parser::bin_ops::this_panics stdout ----
thread 'parser::bin_ops::this_panics' panicked at /home/notyou/.cargo/registry/src/index.crates.io-6f17d22bba15001f/chumsky-0.9.3/src/primitive.rs:945:30:
called `Option::unwrap()` on a `None` value
stack backtrace:
   0: rust_begin_unwind
             at /rustc/bd53aa3bf7a24a70d763182303bd75e5fc51a9af/library/std/src/panicking.rs:665:5
   1: core::panicking::panic_fmt
             at /rustc/bd53aa3bf7a24a70d763182303bd75e5fc51a9af/library/core/src/panicking.rs:74:14
   2: core::panicking::panic
             at /rustc/bd53aa3bf7a24a70d763182303bd75e5fc51a9af/library/core/src/panicking.rs:148:5
   3: core::option::unwrap_failed
             at /rustc/bd53aa3bf7a24a70d763182303bd75e5fc51a9af/library/core/src/option.rs:2015:5
   4: core::option::Option<T>::unwrap
             at /rustc/bd53aa3bf7a24a70d763182303bd75e5fc51a9af/library/core/src/option.rs:965:21
   5: <chumsky::primitive::Choice<[A; N],E> as chumsky::Parser<I,O>>::parse_inner
             at /home/notyou/.cargo/registry/src/index.crates.io-6f17d22bba15001f/chumsky-0.9.3/src/primitive.rs:945:30
   6: chumsky::parse_recovery_inner
             at /home/notyou/.cargo/registry/src/index.crates.io-6f17d22bba15001f/chumsky-0.9.3/src/lib.rs:110:29
   7: chumsky::Parser::parse_recovery
             at /home/notyou/.cargo/registry/src/index.crates.io-6f17d22bba15001f/chumsky-0.9.3/src/lib.rs:201:9
   8: chumsky::Parser::parse
             at /home/notyou/.cargo/registry/src/index.crates.io-6f17d22bba15001f/chumsky-0.9.3/src/lib.rs:241:32
   9: greybolt::parser::bin_ops::this_panics
             at ./src/parser/bin_ops.rs:94:13
  10: greybolt::parser::bin_ops::this_panics::{{closure}}
             at ./src/parser/bin_ops.rs:93:17
  11: core::ops::function::FnOnce::call_once
             at /rustc/bd53aa3bf7a24a70d763182303bd75e5fc51a9af/library/core/src/ops/function.rs:250:5
  12: core::ops::function::FnOnce::call_once
             at /rustc/bd53aa3bf7a24a70d763182303bd75e5fc51a9af/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
@DaniD3v
Copy link
Author

DaniD3v commented Sep 8, 2024

I just found out that this is already fixed on master.
We should probably backport the fix to 0.9

@zesterer
Copy link
Owner

zesterer commented Sep 8, 2024

I agree, this should 'just work'. Monoid laws, etc. I'll look into backporting this over the next few days if it's a problem for you. That said, I'd recommend using 1.0 given that's where all the new dev work is happening.

@DaniD3v
Copy link
Author

DaniD3v commented Sep 9, 2024

I agree, this should 'just work'. Monoid laws, etc. I'll look into backporting this over the next few days if it's a problem for you. That said, I'd recommend using 1.0 given that's where all the new dev work is happening.

I already migrated to 1.0 yesterday. For the 0.9 users I have this little workaround.

if (parsers.len()) > 0 {
    choice(parsers).boxed()
} else {
    empty().not().boxed()
}

What do you think about just releasing a 0.10 version?
I think we're still quite far from 1.0 but the changes since 0.9 are already pretty significant.
I can imagine it's also pretty bothersome to maintain 2 branches all the time.

@zesterer
Copy link
Owner

I'm happy to accept PRs to the 0.9 branch and perform releases for it. I wouldn't consider this to be a breaking change but a fix, too.

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

No branches or pull requests

2 participants