nom icon indicating copy to clipboard operation
nom copied to clipboard

fix: support empty sep for separated_list0 & separated_list1

Open suikammd opened this issue 4 years ago • 3 comments

When separator eats nothing(e.g. opt(tag(","))), separated_list0 & separated_list1 should parse without error. But current implementation returns error when separator eats nothing to prevent infinite loop. I think the infinite loop check should between loops. If no bytes eaten during the whole loop body, then this will result in infinite loop and should return error.

suikammd avatar Jan 22 '22 17:01 suikammd

Seems this will fix #1573 and #1691

SaltyKitkat avatar Nov 14 '23 12:11 SaltyKitkat

If I understand this fix, it changes it from require progress from the sep to requiring progress from either sep or f. Personally, I don't think this is a good idea. While this would allow the caller to choose whether sep or f can be empty, it can also allow both to be empty sometimes and requires either careful reading of the documentation (rarely done) and auditing of the code or exhaustive testing to catch that an infinite loop exists.

A lot of these higher level parsers are relatively trivial (well, barring the 8.0 GAT changes) and people can easily write a parser tailored to their needs. For myself, I only use parsers like this for prototyping but when I'm lining up with a grammar, I use lower level parsers so my code lines up with my BNF grammar.

epage avatar Nov 14 '23 15:11 epage

Just spent a while "discovering" this issue.

The documentation does not indicate there is a "must consume" requirement for the separator parser, causing the resulting error to be surprising. As a result, the type signature is misleading. Perhaps we establish a type to indicate the "must consume" constraint.

Now, in considering the expression, separated_list1(space0, element_parser), it is clear that the list is not technically separated by space0 since it may not exist between elements. As such perhaps a more appropriate solution is a another combinator: maybe_separated_by(optional_separator, element_parser).

gdennie avatar Jan 15 '24 10:01 gdennie