Parser associated types

[Geal]

No description provided.

[epage]

Nice that this lets you drop the phantom data

[Geal]

yes, it simplifies them nicely. I have to check again the methods like flat_map, to make sure that they require all the trait bounds, otherwise it would be possible for them to return a value that does not implement Parser, which would be confusing

[epage]

Any hint if this improved type inference at all? Or any other benefit noticed besides dropping the number of generic parameters / phantom data?

[coveralls]

Pull Request Test Coverage Report for Build 3976558656

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 50 of 57 (87.72%) changed or added relevant lines in 5 files are covered.
• 41 unchanged lines in 4 files lost coverage.
• Overall coverage increased (+0.03%) to 62.135%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/multi/mod.rs	29	32	90.63%
src/internal.rs	3	7	42.86%

Files with Coverage Reduction	New Missed Lines	%
benchmarks/benches/json.rs	1	0.0%
benchmarks/benches/json_streaming.rs	1	0.0%
src/multi/mod.rs	1	90.74%
src/internal.rs	38	21.7%

Totals
Change from base Build 3962072045:	0.03%
Covered Lines:	1554
Relevant Lines:	2501

---

💛 - Coveralls

[Geal]

so far the main issue with type inference was that I had to add the error type explicitely (F: Parser<I, Error=E> instead of F: Parser<I>) in most combinators. It is possible to remove the error generic type, like I've done for many0, but this makes the type signatures harder to read

[epage]

Just gave this a try in winnow-rs/winnow#220 and it looks like the compiler isn't happy about implementing Parser for u8 and other built-ins because there is nothing to constrain the generic E parameter on that is used for the Error associated type.

impl<I, E> Parser<I> for u8
where
    I: StreamIsPartial,
    I: Stream<Token = u8>,
    E: ParseError<I>,
{
    type Output = u8;
    type Error = E;

    fn parse_next(&mut self, i: I) -> IResult<I, u8, E> {
        crate::bytes::one_of(*self).parse_next(i)
    }
}

error[E0207]: the type parameter `E` is not constrained by the impl trait, self type, or predicates
   --> src/parser.rs:606:9
    |
606 | impl<I, E> Parser<I> for u8
    |         ^ unconstrained type parameter

This would require hard coding the error type which would be unpleasant to then make it seemless with other parts of the API.

I guess only O could be turned into an associated type. That would solve most type inference issues. The inconsistency on output types isn't great though.

[epage]

I found doubling down on PhantomData was another way of resolving the type inference issues: winnow-rs/winnow#222