search

jeffreykegler / Marpa--R2

Parse any language you can describe in BNF
https://jeffreykegler.github.io/Marpa-web-site/
Other
157 stars 23 forks source link

Confusion about length of libmarpa token #280

Closed jeffreykegler closed 1 year ago

jeffreykegler commented 2 years ago

Some confusion about the length of a libmarpa token may have propagated upward. The libmarpa macros involved are marpa_v_es_id() and marpa_v_token_start_es_id(). Affected might be the g1length descriptor and MARPA_OP_PUSH_G1_LENGTH in R2.xs. In particular, line https://github.com/jeffreykegler/Marpa--R2/blob/2ef61bdf4d0ff2de3503ca136dbe4a18ff742f9b/cpan/xs/R2.xs#L1806 seems to be wrong.

The solution at this point is probably not to fix R2.xs, which will break everybody depending on the misfeature, but to document that, "for historical reasons", g1length is not what you would expect.

jddurand commented 2 years ago

Jeffrey,

Can you explain the confusion in itself and would be the fix if you were to change to code ?

Thanks, JD.

jeffreykegler commented 2 years ago

Also in error is this line:

https://github.com/jeffreykegler/Marpa--R2/blob/2ef61bdf4d0ff2de3503ca136dbe4a18ff742f9b/cpan/xs/R2.xs#L1798

jeffreykegler commented 2 years ago

@jddurand If I were to change the code, everyone using the g1length action array descriptor for tokens and rules (but not nulling symbols) would see a value that is lesser by one. This would make more sense but would break everything relying on the current behavior.

jeffreykegler commented 1 year ago

Fixed as of https://github.com/jeffreykegler/Marpa--R2/commit/317b920e4b97dff976463dfa68ced346afe8d774. I have added g1len which is equal to the correct length, and deprecated g1length.

Absent feedback to the contrary, I will close this issue after a couple of days.

jeffreykegler commented 1 year ago

Per https://github.com/jeffreykegler/Marpa--R2/issues/280#issuecomment-1347133777, I am closing this issue.