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

Trigger recovery when only recovery stacks are left #2057

Merged
merged 13 commits into from
Oct 24, 2024

Conversation

PieterOlivier
Copy link
Contributor

@PieterOlivier PieterOlivier commented Oct 19, 2024

Previously, error recovery was only triggered when no more active stacks where left.
This PR improves on this by also triggering recovery when only recovery stacks are left.

This change increases recovery quality tremendously, but it also decreased performance significantly.
To combat this, filtering of "duplicate" skip nodes has been added. Only one skip node with for a specific starting position, result type, and skip length is added.
This does not impact recovery quality but it does improve performance dramatically.

The end result of these changes is improved recovery quality and improved performance.

Note that the downside is that the resulting parse forest does not include some nodes. For instance, if we have a language that allows expressions with multiple productions like <expr> + <expr> and <expr> - <expr> and an error occurs before the operator, only one of these expression productions will be included as an error node in the parse tree.
Note that this information can be reconstructed by combining the error nodes with the grammar so it does not really limit the future use cases.

This PR also adds support for limiting the match length. This feature has only been used in experiments but might be actively used in the future so I decided to leave it in.

@PieterOlivier PieterOlivier mentioned this pull request Oct 19, 2024
Copy link

codecov bot commented Oct 19, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 3 lines in your changes missing coverage. Please review.

Project coverage is 49%. Comparing base (2683d8c) to head (4d94b28).
Report is 14 commits behind head on feat/error-recovery.

Files with missing lines Patch % Lines
src/org/rascalmpl/parser/gtd/SGTDBF.java 80% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@                  Coverage Diff                  @@
##             feat/error-recovery   #2057   +/-   ##
=====================================================
  Coverage                     49%     49%           
- Complexity                  6580    6593   +13     
=====================================================
  Files                        685     685           
  Lines                      61127   61147   +20     
  Branches                    8841    8849    +8     
=====================================================
+ Hits                       30189   30215   +26     
  Misses                     28725   28725           
+ Partials                    2213    2207    -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@PieterOlivier PieterOlivier force-pushed the recovery/only-recovery-stacks-left branch from 89c6a5b to 4d94b28 Compare October 24, 2024 05:46
@PieterOlivier PieterOlivier merged commit c75cba6 into feat/error-recovery Oct 24, 2024
7 checks passed
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 this pull request may close these issues.

2 participants