Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing ambiguities when using inline rules #1214

Closed
yurymann opened this issue Nov 16, 2022 · 5 comments · Fixed by #1216
Closed

Missing ambiguities when using inline rules #1214

yurymann opened this issue Nov 16, 2022 · 5 comments · Fixed by #1216

Comments

@yurymann
Copy link

Describe the bug

With some grammars, Earley parser does not return all ambiguities with ambiguity='explicit'.
However, when removing the inlining underscore, all ambiguities are returned.

Potentially related to #536.

To Reproduce

start: _field+

_field: f1 | f2 | f3

f1: INT
f2: INT "M"?
f3: INT "M"

%ignore WS
%import common.WS
%import common.INT

When parsing string 1M 2 with ambiguity='explicit', the result misses ambiguities f2 f2 and f3 f1.

>> t = parser.parse('1M 2')
>> print(t.pretty())

_ambig
  start
    f2	1
    f1	2
  start
    f3	1
    f2	2

However, removing _ from field fixes it, at the expense of the additional tree level which I wanted to avoid.

start: field+

field: f1 | f2 | f3

f1: INT
f2: INT "M"?
f3: INT "M"

%ignore WS
%import common.WS
%import common.INT
>> t = parser.parse('1M 2')
>> print(t.pretty())

start
  _ambig
    field
      f2	1
    field
      f3	1
  _ambig
    field
      f1	2
    field
      f2	2

It's great that the workaround works, at least in this case. But with a complex grammar including inlined rules at several levels, it's difficult to spot the issue before realising that some results are missing in production.

@erezsh
Copy link
Member

erezsh commented Nov 16, 2022

@chanicpanic Any ideas?

@chanicpanic
Copy link
Contributor

Both grammars produce the same SPPF, so the issue is probably in the forest to tree transformation. I will look into it more when I have a chance.

@erezsh
Copy link
Member

erezsh commented Nov 18, 2022

Makes sense! Possibly the inlining of the rules messes with the logic somehow.

chanicpanic added a commit to chanicpanic/lark that referenced this issue Nov 20, 2022
erezsh added a commit that referenced this issue Nov 27, 2022
@yurymann
Copy link
Author

Many thanks for fixing it so quickly! When do you expect to release it?

@erezsh
Copy link
Member

erezsh commented Dec 6, 2022

@yurymann Released 1.1.5 today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants