softdevteam / lrpar

Rust LR parser
Other
1 stars 0 forks source link

Remove unsafe optimisation. #63

Closed ltratt closed 6 years ago

ltratt commented 6 years ago

I originally thought that:

  Insert a, Shift a
  Shift a, Insert a

were equivalent, but they're not: the two sequences might leave the parse in a different state. This "optimisation" thus made us miss minimum cost repair sequences in a handful of cases (not often, but it does happen).

It's not really clear to me whether this is fixable; perhaps we have to do something clever with looking at the parse stacks when we're simplifying repairs?

ltratt commented 6 years ago

Let's hold off on this one for a bit -- it might be the result of another unrelated bug.

ltratt commented 6 years ago

I have resolved this, and the (force pushed) commit hopefully explains better than the PR message why this optimisation is unsound (summary: it used to be sound; but merging compatible nodes broke it).

ptersilie commented 6 years ago

Merged.