-
Notifications
You must be signed in to change notification settings - Fork 69
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
reduce/resuce precedence #51
Conversation
I'd like to add that this is a potentially breaking change (there could be parsers generated differently after the patch). I would appreciate someone's second opinion. |
Thanks for the fix, LGTM. For its being potentially breaking, I believe it should be ok--the users have been warned about the conflict, and then a reduce/reduce conflict is always a thing to take seriously. Also, the next version is going to be 7.0, indicating some breaking would be expected. It would be awesome if you added a test with the grammar from #40. Can you find time to add that? |
Please, please add a command line option to optionally preserve the exact old behaviour. Maintainers: please don't accept this PR until this command line flag is added :) This is to ensure that any downstream users get complete control over whether they adopt this change or not, "just in case". Thank you! |
@enricosada Please note that the F# compiler repo would need to apply this flag, at least in a first instance until a thorough impact analysis has been done |
I'll try to extend this PR to address #39 too. |
Thanks! |
c953c7d
to
f66e283
Compare
I've added a new PR with both fixes and a command line option to preserve previous behaviour. |
Thanks! Normally, you'd put "WIP" into the title of the PR to indicate it is work in progress and not to be merged. |
I've looked into the way fsyacc tries to do error recovery. I'd vote to remove his behaviour. If someone needs it, it's still available with the new --oldprec option. I'll try to find time to add some tests to this commit and get back to you to merge the whole PR. |
2 arguments to control the behavior: --newprec - enable new calculation mode (disabled by default to preserve compatibility) --no-recovery - don't try to reduce as a fallback from invalid input
I've finally found some time to get back to these changes. With neither of the above fsyacc would work as previously (the overal change is not-breaking - you have to opt-in to get the new functionality). |
Closing this old PR |
Path to address:
#40
Some discussion in my previous PR:
#47
Closes #40