added a function that can filter error trees which are in optional positions in a grammar (list elements and optionals) #2074
Conversation
…sitions in a grammar (list elements and optionals)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feat/error-recovery #2074 +/- ##
=====================================================
Coverage 49% 49%
Complexity 6605 6605
=====================================================
Files 685 685
Lines 61148 61148
Branches 8850 8850
=====================================================
+ Hits 30239 30243 +4
+ Misses 28706 28702 -4
Partials 2203 2203 ☔ View full report in Codecov by Sentry. |
| data Tree(int skipped = 0, bool erroneous=false); | ||
|
|
||
| @synopsis{Annotates all nodes of a parse tree with error recovery statistics} | ||
| @description{ |
There was a problem hiding this comment.
I don't know if this is helpful, but disambiguateErrors (implemented in Java) does something similar while disambiguating.
Maybe we can merge the two.
Note that the method you implemented here will bring down the system when a highly ambiguous error tree (not uncommon) is supplied, because sharing will be lost.
I think adding @memo might solve this, but we might want to consider implementing this in Java by reusing some of the disambiguateErrors implementation.
Because of your implementation here I just realized disambiguateErrors currently has a bug because it considers error trees with an empty skipped part as a non-error trees. This has become a bug only after #2053 was merged. I will fix this issue.
There was a problem hiding this comment.
Hi. I added it here as an experiment/demo to see how we can write this kind of code in Rascal. I suspect most of this will be called from Rascal in the future. Good catch with the @memo! We don't have a visit strategy that incorporates memo, so that is really something to watch out for here.
There was a problem hiding this comment.
I think if we merge this we should probably stick with the Java version for now, given that it is much fast in dangerous cases with lots of nested ambiguity.
This could be one of those library functions in Recovery.rsc that is useful to call from a language service. It uses information from grammar rules (so "language-parametric") to decide that the error tree can be ignored in principle.
have to add tests for this of course, before we merge.