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

Implement basic error recovery #2020

Merged
merged 91 commits into from
Sep 23, 2024
Merged

Conversation

PieterOlivier
Copy link
Contributor

This PR implements a basic error recoverer and fixes some issues in the existing error recovery support code.

The new recoverer is called "ToTokenRecoverer" and tries to find a suitable continuation token when recovering from a parse error.

When the parser detects a parse error, the error recover skips part of the input and reduces the production that is currently being recognized. It then let the parser continue.
To determine which part of the input to skip, the recoverer tries to find a literal that could terminate the current production or a literal that could start the next sibling production. If such a literal is found, all input until after the "end" literal or all input until before the "next" literal will be skipped and parsing will continue.

jurgenvinju and others added 30 commits March 30, 2022 16:45
Sometimes recovery nodes start before the current location where
the parser failed to continue. Since the parser works with a
short queue of schedulede TODO's around the current cursor, we
might end up outside of this queue when recovering. This breaks
several unspecified invariants of the SGTBF implementations. For
now I added a detection that a recovery node is to be planned
before the currently retained history and filter that recovery
node. The next step will be to make sure backtracking over the
current location is made possible.
… because the next parser loop iteration always wants to advance one character
…e version of Rascal and (b) the edit command is wired to the edit IDEService
…t in the scheme by copyinhthe contents to a tmp file
Copy link

codecov bot commented Aug 29, 2024

Codecov Report

Attention: Patch coverage is 28.40419% with 1162 lines in your changes missing coverage. Please review.

Project coverage is 49%. Comparing base (c56a6fd) to head (5a2266b).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...rascalmpl/util/visualize/ParseStateVisualizer.java 3% 319 Missing ⚠️
...scalmpl/parser/uptr/recovery/ToTokenRecoverer.java 0% 204 Missing ⚠️
src/org/rascalmpl/parser/gtd/SGTDBF.java 57% 51 Missing and 14 partials ⚠️
src/org/rascalmpl/util/visualize/dot/NodeId.java 10% 63 Missing ⚠️
src/org/rascalmpl/util/visualize/dot/DotGraph.java 0% 56 Missing ⚠️
...org/rascalmpl/util/visualize/dot/DotAttribute.java 0% 51 Missing ⚠️
...r/uptr/recovery/CaseInsensitiveLiteralMatcher.java 0% 34 Missing ⚠️
src/org/rascalmpl/util/visualize/dot/DotNode.java 0% 32 Missing ⚠️
src/org/rascalmpl/util/visualize/dot/DotEdge.java 0% 29 Missing ⚠️
.../rascalmpl/parser/gtd/stack/SkippingStackNode.java 29% 23 Missing and 1 partial ⚠️
... and 47 more
Additional details and impacted files
@@            Coverage Diff            @@
##              main   #2020     +/-   ##
=========================================
- Coverage       49%     49%     -1%     
- Complexity    6298    6490    +192     
=========================================
  Files          664     685     +21     
  Lines        59626   61060   +1434     
  Branches      8646    8880    +234     
=========================================
+ Hits         29476   29990    +514     
- Misses       27939   28810    +871     
- Partials      2211    2260     +49     

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

@PieterOlivier PieterOlivier changed the base branch from main to error-recovery September 4, 2024 06:59
@PieterOlivier PieterOlivier marked this pull request as ready for review September 4, 2024 07:00
Copy link

@sungshik sungshik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that's an impressive amount of work! I'm very curious to see all this working in VS Code down the line 🙂 For now, I think none of my comments are blocking. Lots of minor nits, and other than that, just a bunch of questions about certain implementation choices and small suggestions. In general, the approach looks sound to me.

src/org/rascalmpl/library/ParseTree.rsc Outdated Show resolved Hide resolved
src/org/rascalmpl/library/ParseTree.rsc Outdated Show resolved Hide resolved
src/org/rascalmpl/library/ParseTree.rsc Outdated Show resolved Hide resolved
src/org/rascalmpl/library/ParseTree.rsc Show resolved Hide resolved
src/org/rascalmpl/library/ParseTree.rsc Outdated Show resolved Hide resolved
src/org/rascalmpl/util/visualize/ParseStateVisualizer.java Outdated Show resolved Hide resolved
src/org/rascalmpl/util/visualize/ParseStateVisualizer.java Outdated Show resolved Hide resolved
src/org/rascalmpl/util/visualize/dot/NodeId.java Outdated Show resolved Hide resolved
Copy link
Member

@DavyLandman DavyLandman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've also gone through and added some questions/remarks.

Impressive piece of work 👍

pom.xml Show resolved Hide resolved
src/org/rascalmpl/library/ParseTree.rsc Outdated Show resolved Hide resolved
src/org/rascalmpl/library/ParseTree.rsc Show resolved Hide resolved
src/org/rascalmpl/unicode/UnicodeConverter.java Outdated Show resolved Hide resolved
src/org/rascalmpl/util/visualize/replay.html Outdated Show resolved Hide resolved
@PieterOlivier PieterOlivier merged commit a66cca4 into error-recovery Sep 23, 2024
@PieterOlivier PieterOlivier deleted the to-token-recoverer branch September 23, 2024 09:30
@DavyLandman DavyLandman mentioned this pull request Sep 24, 2024
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.

4 participants