Skip to content

Conversation

@jbdoderlein
Copy link
Contributor

Moved from this PR on rascal-lsp repo

This PR add a possible implementation of 3 new command to the Rascal DAP implementation : evaluate, setVariable and completions.
In VSCode, these new commands allow interacting with the Rascal Interpreters inside the Debug Console, and allow changing values of locals variable in the debug panel.

2025-11-17.15-19-19.mp4

I implemented the evaluation so that it runs in the same interpreter as the debugger, which requires disabling the EventTrigger during evaluation to avoid triggering a breakpoint.

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.

This looks quite good, I've done a first pass where I looked at the DAP code, there is some internal stuff about the interpreter that I want to have @jurgenvinju take a look at before we would merge this.

@jurgenvinju
Copy link
Member

Ok thanks both. I'll try and study this tomorrow.

@jbdoderlein
Copy link
Contributor Author

Thank you for your comments @DavyLandman !

I've almost fixed all concern, the one I'm not sure is about error handling.

I added a output event on RuntimeException, so it's displayed nicely in the debug console.
image

Should this be dealt with elsewhere?

@jbdoderlein
Copy link
Contributor Author

Also, it seems likes there is some race condition on watch :
The watch expressions are loaded sometimes before the variables are computed, so when the debugger hit the first breakpoints there is no variable reference and the new evaluation code is used.

For instance, this is the state when launching main function :
image

then after adding new watch expression (= trigerring the computation of all current watch) :

image

@DavyLandman
Copy link
Member

hi @jbdoderlein

I added a output event on RuntimeException, so it's displayed nicely in the debug console.
Should this be dealt with elsewhere?

Yes, that is one way to deal with it. I think we could also reuse the logic inside of ReadEvalPrintDialogMessages that is able to deal with a set of exceptions and print it, this is also used in the REPL.

catch (InterruptException ex) {
return printer.outputError((w, sw, u) -> {
w.println((u ? "»» " : ">> ") + "Interrupted");
ex.getRascalStackTrace().prettyPrintedString(w, sw);
});
}
catch (RascalStackOverflowError e) {
return printer.outputError((w, sw, _u) -> {
throwMessage(w, e.makeThrow(), sw);
});
}
catch (StaticError e) {
return printer.outputError((w, sw, _u) -> {
staticErrorMessage(w, e, sw);
});
}
catch (Throw e) {
return printer.outputError((w, sw, _u) -> {
throwMessage(w,e, sw);
});
}
catch (QuitException q) {
throw new StopREPLException();
}
catch (Throwable e) {
if (e instanceof ParseError) {
throw e;
}
return printer.outputError((w, sw, _u) -> {
throwableMessage(w, e, eval.getStackTrace(), sw);
});
}

Also, it seems likes there is some race condition on watch :

We've seen something similar in rascal-lsp, and it's quite finicky but it can be related to how requests that come in over DAP are scheduled on the workerpool for lsp4j. This PR is fixing that: usethesource/rascal-language-servers#893

It's a combination of a single threaded thread pool for dap/lsp4j request handling and then running code that needs to be linear to the order of incoming request on that handler, and scheduling long-running jobs on our own threadpool that is allowed to spinup multiple threads.

It might be a fix we also need here?

@jurgenvinju
Copy link
Member

Thank you for your comments @DavyLandman !

I've almost fixed all concern, the one I'm not sure is about error handling.

I added a output event on RuntimeException, so it's displayed nicely in the debug console. image

Should this be dealt with elsewhere?

This debug console is mimicking REPL behavior and from that we can either reuse or steel the proper behavior. From RascalInterpreterREPL:

@Override
    public ICommandOutput handleInput(String command) throws InterruptedException, ParseError, StopREPLException {
        var result = handleDebuggerCommand(command);
        if (result != null) {
            return result;
        }
        Objects.requireNonNull(eval, "Not initialized yet");
        synchronized(eval) {
            try {
                Set<String> changes = new HashSet<>();
                changes.addAll(dirtyModules);
                dirtyModules.removeAll(changes);
                eval.reloadModules(eval.getMonitor(), changes, URIUtil.rootLocation("reloader"));
                return printer.outputResult(eval.eval(eval.getMonitor(), command, PROMPT_LOCATION));
            }
            catch (InterruptException ex) {
                return printer.outputError((w, sw, u) -> {
                    w.println((u ? "»» " : ">> ") + "Interrupted");
                    ex.getRascalStackTrace().prettyPrintedString(w, sw);
                });
            }
            catch (RascalStackOverflowError e) {
                return printer.outputError((w, sw, _u) -> {
                    throwMessage(w, e.makeThrow(), sw);
                });
            }
            catch (StaticError e) {
                return printer.outputError((w, sw, _u) -> {
                    staticErrorMessage(w, e, sw);
                });
            }
            catch (Throw e) {
                return printer.outputError((w, sw, _u) -> {
                    throwMessage(w,e, sw);
                });
            }
            catch (QuitException q) {
                throw new StopREPLException();
            }
            catch (Throwable e) {
                if (e instanceof ParseError) {
                    throw e;
                }
                return printer.outputError((w, sw, _u) -> {
                    throwableMessage(w, e, eval.getStackTrace(), sw);
                });
            }
        }
    }

Especially RascalStackOverflowError is interesting, but here you also see ParseError and StaticError for when the user types something wrong.

ParseError is handler higher up in the REPL stack, in the generic part of RascalReplServices:

@Override
    public ICommandOutput handleInput(String input) throws InterruptedException, StopREPLException {
        try {
            return lang.handleInput(input);
        }
        catch (ParseError pe) {
            return ParseErrorPrinter.parseErrorMaybePrompt(pe, lang.promptRootLocation(), input, term, prompt(false, unicodeSupported).length() + 1);
        }
    }

So you can reuse or clone ParseErrorPrinter for that.

@jbdoderlein it is also conceiveable that you reuse a RascalInterpreterREPL object and link it to the streams you have. TutorCommandExecutor is an example of how that can be done. This thing simulates a REPL for producing documentation strings in the website. It has its own way of loading an Evaluator and configuring it. You could make a simpler version. Not necessary, but something to consider.

@jbdoderlein
Copy link
Contributor Author

Ok yes I think I should rewrite as a command executor. This should simplify and clarify the error handling.

For now I updated the current branch to remove setVariable requests, as it may need a bit more rewriting of the SuspendedState class (we need to have a way to get the environment of a variable from it's variable reference).

@jurgenvinju I now use new Environment like you suggested and it works fine. Now evaluate work for all stackframes available.

I also removed the code that try to provide variable directly as response to watch requests because it used a frameID as a scopeID, which only worked because the first scope is initialized at 0.

@jbdoderlein
Copy link
Contributor Author

jbdoderlein commented Nov 21, 2025

We've seen something similar in rascal-lsp, and it's quite finicky but it can be related to how requests that come in over DAP are scheduled on the workerpool for lsp4j. This PR is fixing that: usethesource/rascal-language-servers#893

It's a combination of a single threaded thread pool for dap/lsp4j request handling and then running code that needs to be linear to the order of incoming request on that handler, and scheduling long-running jobs on our own threadpool that is allowed to spinup multiple threads.

It might be a fix we also need here?

I found the underlying issue : there is no specification in DAP on the order of variables and watch requests. The problem was that the variableReferenceId are associated on scopes/variables requests, and vscode call evaluate for Watch before.
Since the code that use variable for watch is removed, the issue doesn't exist anymore, but the initialization of variable reference should maybe happen on suspend state (before any requests are handled)

@jbdoderlein
Copy link
Contributor Author

Ok I refactored the evaluation part to output ICommandOutput with result.

Now errors are clear, and I even reused ParseError printer.
image

@codecov
Copy link

codecov bot commented Nov 21, 2025

Codecov Report

❌ Patch coverage is 0% with 195 lines in your changes missing coverage. Please review.
✅ Project coverage is 47%. Comparing base (570bd8e) to head (6502711).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/org/rascalmpl/dap/RascalDebugAdapter.java 0% 128 Missing ⚠️
src/org/rascalmpl/debug/DebugHandler.java 0% 63 Missing ⚠️
.../org/rascalmpl/debug/IRascalRuntimeEvaluation.java 0% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##              main   #2523    +/-   ##
========================================
- Coverage       47%     47%    -1%     
- Complexity    6709    6711     +2     
========================================
  Files          792     793     +1     
  Lines        65344   65521   +177     
  Branches      9784    9812    +28     
========================================
+ Hits         30932   30947    +15     
- Misses       31989   32154   +165     
+ Partials      2423    2420     -3     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jbdoderlein
Copy link
Contributor Author

I refactored the evaluation part inside the debug handler. This will allow in next PR to implement conditional breakpoint using the same infrastructure for expression evaluation.

I think the code is stable enough to be merged.

Copy link
Member

@jurgenvinju jurgenvinju left a comment

Choose a reason for hiding this comment

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

This is very nice. I have some small suggestions to improve the code, but otherwise it is good.

@jbdoderlein
Copy link
Contributor Author

@jurgenvinju I implemented all the suggestions/fix

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.

5 participants