winnow-rs / winnow

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

trace() in function returning impl Parser triggers rustc limitations #494

Closed diskdance closed 8 months ago

diskdance commented 8 months ago

Please complete the following tasks

rust version

rustc 1.75.0 (82e1608df 2023-12-21)

winnow version

0.6.2

Minimal reproducible code

pub fn parser(
    some_config_used_by_the_parser: u8,
) -> impl for<'i> Parser<&'i str, char, ContextError> {
    trace("parser", move |input: &mut &str| {
        // Some complex logic here...
        todo.parse_next(input)
    })
}

Steps to reproduce the bug with the above code

Compile it.

Actual Behaviour

Compiler error:

error[E0277]: the trait bound `for<'i> impl winnow::Parser<&str, _, _>: winnow::Parser<&'i str, char, winnow::error::ContextError>` is not satisfied
  --> example/src/main.rs:13:6
   |
13 | ) -> impl for<'i> Parser<&'i str, char, ContextError> {
   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `for<'i> winnow::Parser<&'i str, char, winnow::error::ContextError>` is not implemented for `impl winnow::Parser<&str, _, _>`
   |
   = help: the following other types implement trait `winnow::Parser<I, O, E>`:
             <char as winnow::Parser<I, char, E>>
             <u8 as winnow::Parser<I, u8, E>>
             <std::boxed::Box<(dyn winnow::Parser<I, O, E> + 'a)> as winnow::Parser<I, O, E>>
             <winnow::combinator::Repeat<P, I, O, C, E> as winnow::Parser<I, C, E>>
             <winnow::combinator::ByRef<'p, P> as winnow::Parser<I, O, E>>
             <winnow::combinator::Map<F, G, I, O, O2, E> as winnow::Parser<I, O2, E>>
             <winnow::combinator::TryMap<F, G, I, O, O2, E, E2> as winnow::Parser<I, O2, E>>
             <winnow::combinator::VerifyMap<F, G, I, O, O2, E> as winnow::Parser<I, O2, E>>
           and 43 others
note: this is a known limitation of the trait solver that will be lifted in the future
  --> example/src/main.rs:13:6
   |
13 | ) -> impl for<'i> Parser<&'i str, char, ContextError> {
   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ try adding turbofish arguments to this expression to specify the types manually, even if it's redundant

Expected Behaviour

Should compile.

Additional Context

The compiler says "adding turbofish arguments to this expression" but AFAIK there is nowhere to put it in that expression, and trace() uses impl in its parameters so manually annotating a parser type is not possible. Probably this can be fixed by adding a named type parameter for Parser?

Also, removing the trace() wrapper will make it compile.

epage commented 8 months ago

FYI the following compiles:

use winnow::prelude::*;
use winnow::error::ContextError;
use winnow::combinator::trace;
use winnow::combinator::todo;

pub fn parser<'i>(
    some_config_used_by_the_parser: u8,
) -> impl Parser<&'i str, char, ContextError> {
    trace("parser", move |input: &mut &'i str| {
        // Some complex logic here...
        todo.parse_next(input)
    })
}

(lifetime parameter rather than HRTB)

diskdance commented 8 months ago

FYI the following compiles:

use winnow::prelude::*;
use winnow::error::ContextError;
use winnow::combinator::trace;
use winnow::combinator::todo;

pub fn parser<'i>(
    some_config_used_by_the_parser: u8,
) -> impl Parser<&'i str, char, ContextError> {
    trace("parser", move |input: &mut &'i str| {
        // Some complex logic here...
        todo.parse_next(input)
    })
}

(lifetime parameter rather than HRTB)

Yes, and actually my code was originally written in this form and later changed to the HRTB form. That's because HRTB looks more semantically correct (Parser should accept any &str from caller instead of having it inferred from parser()). Anyway, I'm still learning Rust and trying to figure out all the lifetime stuffs, so correct me if I was wrong :)

By submitting this issue I mean this could be probably solved by changing trace()'s signature so it's compatible with (even) HRTB.

epage commented 8 months ago

As someone programming in Rust for years, I almost never use HRTB :)

I'm not seeing anything obvious to change about trace or todo to make it compatible with HRTB

diskdance commented 8 months ago

Just found that wrapping in another closure will make HRTB work:

pub fn parser(
    some_config_used_by_the_parser: u8,
) -> impl for<'i> Parser<&'i str, char, ContextError> {
    |input: &mut &str| {
        trace("parser", |input: &mut &str| {
            // Some complex logic here...
            todo.parse_next(input)
        }).parse_next(input)
    }
}

Thanks for the reply anyway.