rascal
rascal copied to clipboard
This fixes #1920 but it has a backward compatibility problem.
- stackoverflow errors were not reported properly. Instead the interpreter triggered many more stackoverflow errors while trying to report on them.
- fixed this by capturing the deepest overflow exception and wrapping the current runtime stack in a cheap exception object. This object is then thrown an caught by the REPL loop which prints the proper exception stack trace.
- We lost the ability to catch a stackOverflow() exception in Rascal code with this. This is problematic since there are tools that use that (drAmbiguity) in case of expected eternal recursions. So for now this is PR and I would like to hear if anybody has ideas on how to fix this properly without loss of this important feature.
At least this fix confirms the diagnosis that the error handler was causing new StackOverflowErrors, which explains the messages printed here #1920.
Maybe we can find a proper stack frame deeper in than the REPL is to throw the real throw.
Codecov Report
Attention: Patch coverage is 5.88235% with 16 lines in your changes are missing coverage. Please review.
Project coverage is 49%. Comparing base (
f34dc15) to head (767ea0b). Report is 18 commits behind head on main.
:exclamation: Current head 767ea0b differs from pull request most recent head 6595ea8. Consider uploading reports for the commit 6595ea8 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## main #1921 +/- ##
=======================================
- Coverage 49% 49% -1%
+ Complexity 6165 6163 -2
=======================================
Files 661 662 +1
Lines 58702 58711 +9
Branches 8547 8548 +1
=======================================
- Hits 28965 28961 -4
- Misses 27545 27561 +16
+ Partials 2192 2189 -3
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@PaulKlint I'm not sure if we could implement catchable stackOverlflow() exceptions anyway in the compiled context. There is no test for it, even though I've used it in the past. If we can't implement it, we might as well bite the bullet now and remove that feature from the interpreter too. WDYT?