Questions about error handling

[Phaiax]

Hi @Marwes

I'm hitting the error part in https://github.com/Marwes/combine/wiki/Documentation-Draft and I have some questions regarding the overall design.

Why is the End-Of-File error handled by the Error traits/types? Wouldn't it be better to handle it right with Consumed<> or so?

Next is about Consumed::Empty. I think it means that the parser has not consumed any bytes from the input (or that he has reset() the input to the original position). Why is it called Empty and not Nothing or Unchanged or so. Am I missing the point?

FastResult is a flattened ParseResult. But why is there a unflattened version when all user facing functions parse(), easy_parse() .... aaa, parse_stream() never asked...

If I write out ConsumedResult it becomes

type ConsumedResult<O, I> = FastResult<O, <I as StreamOnce>::Error>;
    ConsumedOk(O)
    EmptyOk(O)
    ConsumedErr(I::Error)
    EmptyErr( Tracked { error : I::Error, offset : u8 })

Why is EmptyErr Tracked and ConsumedErr not? Is ConsumedErr always not recoverable and therefore don't need the Tracking?

Why ParseMode and is_partial? They do the same, except that ParseMode monomorphizes. I guess because carrying another associated type from StreamOnce would be noisy?

Thanks a lot, Daniel

[Marwes]

Thanks for doing the work on the wiki! I will try and read it over as soon as I have a chance!

Why is the End-Of-File error handled by the Error traits/types? Wouldn't it be better to handle it right with Consumed<> or so?

Maybe 🤔 . The special EOF handling were added after 3.0 was released so wouldn't have been able to modify Consumed for it then. Maybe for 4.x though.

One potential problem with moving it to Consumed is that it would add another variant to Consumed/FastResult which could cause a slowdown in parsing. As it is now, eof checking is only needed for partial parsing and it is kept out of the fast path which I think helps keep everything as fast as possible.

Next is about Consumed::Empty. I think it means that the parser has not consumed any bytes from the input (or that he has reset() the input to the original position). Why is it called Empty and not Nothing or Unchanged or so. Am I missing the point?

Those are better names, yeah. I such at naming. Should change for 4.x (they have been around since 0.x and I have never thought about it).

FastResult is a flattened ParseResult. But why is there a unflattened version when all user facing functions parse(), easy_parse() .... aaa, parse_stream() never asked...

Backwards compatibility and it allows users to use ?/try!. It might be worth removing perhaps, dunno.

Why is EmptyErr Tracked and ConsumedErr not? Is ConsumedErr always not recoverable and therefore don't need the Tracking?

Tracked is an implementation detail I never really fleshed out, I have tried to keep it hidden from users as much as possible. It keeps track of how many parsers in a sequence have returned EmptyOk so the parser can backtrack and collect the errors for those parsers. I need to document it internally but users should ideally ignore it.

Why ParseMode and is_partial? They do the same, except that ParseMode monomorphizes. I guess because carrying another associated type from StreamOnce would be noisy?

It makes it easy to implement both the first parse (which knows it does not need to restore from any partial state) and the partial parser (which needs to inspect the partial state to determine from where to continue).

This is also something I have kept a bit internal as I wasn't 100% if this was the best way of doing it.

[Marwes]

@Phaiax Is the latest revision at https://github.com/Marwes/combine/wiki or https://github.com/Phaiax/combine/wiki ?

[Phaiax]

Latest is Marwes/combine/wiki

Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.