let-def / lrgrep

Menhir polishing toolbox, for experienced druids
25 stars 2 forks source link

Reductions with empty productions #12

Closed SquidDev closed 10 months ago

SquidDev commented 10 months ago

Apologies for the slew of issues here (though thank you for all the work you've done on fixing them! :heart:).

I'm continuing to update my current code to use the latest version of lrgrep, and have hit a problem where reductions for empty productions do not appear to be matched.

I've committed a small reproduction case, but just to explain what's going on here.

We have the following grammar which accepts any number of characters between two brackets (e.g. (), (a), (aaa)).

let sentence := "(" ; ~ = chars ; ")" ; EOF ; <>

let chars := ~ = list(char) ; <List.rev>
let char := ~ = C ; <>

I'd like to match sentences without a closing bracket, and so have the following rule:

rule error_message = parse error
| OPAREN ; [chars]
{ "Unclosed '('" }

While this does match sentences like (aaa, it does not match just (. Curiously this does work with LRgrep 2 (so using OPAREN ; chars ; !), so I'm assuming this is a regression rather than intentional behaviour?

Sorry I can't provide more info! Still going down the rabbit hole of trying to understand how lrgrep works.

let-def commented 10 months ago

Thanks for the bug report (and cheers to 2024! :) ).

This is indeed a regression. I thought I had handled that case but it was subtler than I expected...

Long story short: when initiating a reduction there is a special case (a "limit condition") because we don't know anything about the state of the stack yet, so we have to peek at the first state, which is not part of the reduction but allow us to identify the possible reductions coming next. And the code was not handling this case correctly, because it forgot to "unconsume" this first state (the corresponding expression would have been something like OPAREN ; [OPAREN; chars], which is completely incorrect but is what the code was trying to match when chars is empty). Yet another form of off-by-one :).

And looking at it again, I got an idea that might make it possible to get rid of this special case, I will experiment...

let-def commented 10 months ago

And thanks for the tiny example, I will keep it in the code base it is very convenient for experimenting.

let-def commented 10 months ago

I think I found a way to simplify this case, the current master has a more uniform treatment of reductions.

SquidDev commented 10 months ago

Thank you so much - just tried out current master and it works perfectly! Really liking the new syntax, I've managed to get rid of all my hacks, and there's some rules which are significantly cleaner now.

Happy new year to you too! :)