winnow-rs / winnow

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

[Tutorial](https://docs.rs/winnow/latest/winnow/_tutorial/chapter_6/index.html) implies ParseError is std::error:Error, but it is not #575

Closed rbtcollins closed 3 months ago

rbtcollins commented 3 months ago

Please complete the following tasks

rust version

1.79

winnow version

v0.6.16

Minimal reproducible code


#[derive(ThisError)]
pub enum Error {
    #[error("Data error {}", _0)]
    DataError(#[from] winnow::error::ParseError<&'static [u8], &'static [u8]>), // 'static is wrong, this just helps isolate the example
}

Steps to reproduce the bug with the above code

compile it

Actual Behaviour

the method `as_dyn_error` exists for reference `&ParseError<&[u8], &[u8]>`, but its trait bounds were not satisfied
the following trait bounds were not satisfied:
`winnow::error::ParseError<&[u8], &[u8]>: std::error::Error`
which is required by `winnow::error::ParseError<&[u8], &[u8]>: AsDynError<'_>`
`&winnow::error::ParseError<&[u8], &[u8]>: std::error::Error`
which is required by `&winnow::error::ParseError<&[u8], &[u8]>: AsDynError<'_>`

Expected Behaviour

thiserror's from implementation should be able to handle ParseError as the intended interop-with-rust-ecosystem type.

Additional Context

The discussions around this so far suggest converting to a String at the call sight, which is certainly a viable workaround, but if so, please explain that in the tutorial page that highlights the importance of an owned std::error::Error to ecosystem interop

epage commented 3 months ago

Yes, that was left in between the lines which we should make more explicit. #576 tries to make this more explicit.

The reason it isn't directly compatible is so you can make the decision on your side for how you want to handle it (and things are a bit messier with generics).