rascal icon indicating copy to clipboard operation
rascal copied to clipboard

This fixes #1920 but it has a backward compatibility problem.

Open jurgenvinju opened this issue 1 year ago • 3 comments

  • 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.

jurgenvinju avatar Feb 29 '24 15:02 jurgenvinju

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.

jurgenvinju avatar Feb 29 '24 15:02 jurgenvinju

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

Files Patch % Lines
...rascalmpl/exceptions/RascalStackOverflowError.java 0% 11 Missing :warning:
src/org/rascalmpl/repl/RascalInterpreterREPL.java 0% 3 Missing :warning:
...rc/org/rascalmpl/semantics/dynamic/Expression.java 33% 2 Missing :warning:
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.

codecov[bot] avatar Feb 29 '24 15:02 codecov[bot]

@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?

jurgenvinju avatar Feb 29 '24 15:02 jurgenvinju