Skip to content

[Priroda] Add minimal DAP frontend with single-thread stepping demo - #5241

Merged
oli-obk merged 34 commits into
rust-lang:masterfrom
moabo3li:priroda-dap-startup
Aug 5, 2026
Merged

[Priroda] Add minimal DAP frontend with single-thread stepping demo#5241
oli-obk merged 34 commits into
rust-lang:masterfrom
moabo3li:priroda-dap-startup

Conversation

@moabo3li

@moabo3li moabo3li commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Adds a DAP frontend so Priroda can be driven by VS Code, nvim-dap, or
any Debug Adapter Protocol editor.

What's working:

  • Initialize/launch/configurationDone handshake with lifecycle state
    tracking and request-id validation
  • Single-thread step/next/continue with stopped events
  • Interpreter exit (normal/abort/UB) → terminated/exited events
  • threads, stackTrace, scopes, variables, and setBreakpoints
  • Macro spans resolved to callsite for correct breakpoint/step locations
  • Unbounded request loop (was previously capped at 128)

Code restructure:

  • src/debugger.rs → shared PrirodaContext with
    stop-at-first-user-location and breakpoints
  • src/frontend/cli.rs → extracted CLI rendering
  • src/frontend/dap.rs → new DAP frontend using emmy_dap_types
  • create_ecx errors reported instead of panicking

Testing:

  • 12 DAP ui_test fixtures (.rs + .stdin transcript + .stdout) covering
    handshake, data views, stepping, breakpoints, and negative protocol
    cases.

@rustbot rustbot added the S-waiting-on-review Status: Waiting for a review to complete label Aug 3, 2026
@moabo3li
moabo3li force-pushed the priroda-dap-startup branch from 3b13771 to 7e6411b Compare August 3, 2026 13:22
Comment thread priroda/src/main.rs Outdated
Comment thread priroda/src/frontend/dap.rs Outdated
Comment thread priroda/src/frontend/dap.rs
Comment thread priroda/src/frontend/dap.rs Outdated
|| self.require_stopped(&request)?
|| self.require_thread_id(&request)?
{
return Ok(());

@oli-obk oli-obk Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it right to return Ok here? Could also bubble up a special "invalid command" error

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The require_* methods already send an error response to the client before returning, so Ok(()) means "already handled." Bubbling up an error would double-respond on the same request_seq.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

hm. but you could solely bubble up the error and not eagerly report it within the require method

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, all require_* helpers return pure Result<(), &'static str> and none of them call respond or send_event, errors bubble via ? up to run_requests which is the single site that calls request.error(msg), no double-respond risk

Comment thread priroda/src/frontend/dap.rs Outdated
Comment thread priroda/src/frontend/dap.rs Outdated
Comment thread priroda/src/frontend/dap.rs Outdated
@rustbot rustbot removed the S-waiting-on-review Status: Waiting for a review to complete label Aug 4, 2026
@rustbot

rustbot commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot added the S-waiting-on-author Status: Waiting for the PR author to address review comments label Aug 4, 2026
@moabo3li
moabo3li force-pushed the priroda-dap-startup branch 2 times, most recently from a4f989d to ead5b3e Compare August 4, 2026 17:16
@rustbot

This comment has been minimized.

moabo3li added 21 commits August 5, 2026 14:40
Move command-result printing into a helper and keep CLI loop control at the call site.
Consume Priroda's --dap flag before handing arguments to rustc_driver::run_compiler, then dispatch the freshly-created PrirodaContext to either the existing CLI loop or a new DAP loop stub.
Add FirstUserSourceLocation ResumeMode variant that stops when the
interpreter reaches a user-relevant frame with a source location.
This gives the DAP frontend an entry-stop primitive that skips
Miri-internal and std frames.
Wire `next` and `stepIn` DAP requests to Priroda's existing source-line
step.  Both commands use the same `handle_step` handler for now; true
step-over vs step-in semantics are deferred.

Add `stopped_reason` to map `StepResult` variants to DAP
`StoppedEventReason` so the editor can distinguish a manual step from a
breakpoint hit.  Add a `handle_disconnect` handler that sends the
`terminated` event and exits the session cleanly.

Document the `SourceLocation` span-storage rationale and refine the
`SourceLine` resume-mode comment to be clearer about the "no source
location → first mapped location" semantics.
When the session receives an unsupported DAP request, return
`DispatchOutcome::Continue` instead of `Exit` so the debug adapter keeps
running after sending the error response.  Remove the `eprintln!` side
channel from `handle_unsupported_request` since the framed DAP error
response is the single authoritative error-reporting path.

Include the command name in the error message string so the DAP client
sees which request was rejected.
Replace `unreachable!()` with `bug!(...)` at the four dispatch-guaranteed
invariant sites so they produce a meaningful message when the guard fails
instead of a bare panic.  Also switch the DAP error print to Debug format
so transport errors include their chain.
Call `span.source_callsite()` in `resolve_current_location` so
breakpoints and source reporting use the user-visible macro call site
instead of the expanded macro body for lines generated by `println!`,
`assert_eq!`, and similar macros.
Add the `continue` command handler, reusing the existing source-line
stepping and breakpoint infrastructure.  The `handle_continue` method
follows the same `ExecutionOutcome` dispatch pattern as `handle_step`.

Include `Command::Continue` in `require_thread_id` validation so the
request passes the thread-id guard, and widen the fallback from
`unreachable!()` to `true` so any future request with a thread-id field
passes validation rather than panicking.
Add the `setBreakpoints` command handler that maps DAP source breakpoints
to the shared `PrirodaContext::set_breakpoint` breakpoint table.  Every
requested breakpoint is marked as verified so that VS Code displays
the breakpoint marker in the editor gutter; path and line-range
validation is deferred per the existing FIXME in `debugger.rs`.
Mark `supportsSingleThreadExecutionRequests: true` so that VS Code sends
the `singleThread` flag on step/continue requests.  This lets the editor
drive the single-threaded prototype without protocol errors.

Remove the `MAX_REQUEST_COUNT` guard and the `for 0..MAX_REQUEST_COUNT`
loop, replacing them with a simple `loop {}`.  The debug adapter now
handles an unbounded number of requests, terminating only on disconnect
or an explicit exit event.

Set `source_reference: Some(0)` on stack frames so the editor does not
request source content through `source` requests: the file is on disk
and the editor can read it directly.

Update all DAP `.stdout` fixture files to reflect the new capability
field in the `initialize` response body.
DAP Content-Length headers embed the byte count of the following JSON,
which drifts after path normalisation replaces the real manifest dir
with {MANIFEST_DIR}.  Replace Content-Length values with a
{CONTENT_LENGTH} placeholder so path-length differences between
machines do not cause spurious Content-Length mismatches in CI.
@moabo3li
moabo3li force-pushed the priroda-dap-startup branch from ead5b3e to 6574965 Compare August 5, 2026 11:46
@rustbot

rustbot commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@moabo3li
moabo3li force-pushed the priroda-dap-startup branch 2 times, most recently from 53d7ead to 2bf388e Compare August 5, 2026 12:08

@oli-obk oli-obk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the general error handling needs an overhaul. You're doing a lot of redundant checks and handling Result<bool, E> is almost always an anti-pattern.

View changes since this review

Comment thread priroda/src/frontend/dap.rs Outdated
Comment thread priroda/src/frontend/dap.rs Outdated

fn handle_scopes<'tcx>(
&mut self,
request: Request,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah I didn't realize the request was so necessary everywhere. Well it kinda isn't strictly. Ideally we wouldn't have to pass it in at all, but handle success/error outside and just return a Result<ResponseBody, String> or sth

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, handlers return Result<HandlerSuccess, &'static str> with no Request param and run_requests does both the success (request.success(body)) and error (request.error(msg)) sends, state transitions and event forwarding happen after the response send so a transport failure leaves session state untouched, wire output is unchanged except the repeated configurationDone fixture message text

Comment thread priroda/src/frontend/dap.rs Outdated
@moabo3li
moabo3li force-pushed the priroda-dap-startup branch from 2bf388e to e301817 Compare August 5, 2026 17:05
List every Command variant in dispatch_request and display_command
instead of a `_ =>` catch-all.  

New variants added upstream then fail to compile here instead of silently falling through the unsupported arm.
…ts path guard

Pull the inner arguments out of request.command at dispatch time and
pass them by value into handle_scopes / handle_variables /
handle_set_breakpoints, so the handlers no longer re-match on
request.command.  require_frame_id and require_variables_reference
take the extracted value; the bug! fallback for the dispatch-only
command in handle_variables is gone.

Replace the inline DapState::Fresh check in dispatch_request with a
require_initialized predicate, mirroring the other require_* guards.

Reject setBreakpoints with an error when source.path is missing --
Priroda only resolves file-based breakpoints.
The Locals scope carried no source/line/column, so the editor could not anchor the variables view to the stopped frame.  

Pull them from session.current_location when present and bless the dap_scopes_variables* fixtures to the new fields.
Convert every require_* and reject_after_termination predicate from
ServerResult<bool> eager-respond to pure Result<(), &str>, add DispatchOutcome::Rejected(&str), and change handlers to return
Result<DispatchOutcome, ServerError>.  

Once predicates stop eagerly responding, Rejected carries their errors out -- and vice versa.

dispatch_request return type becomes InterpResult<Result<DispatchOutcome, ServerError>>.  

run_requests clones the request before dispatch so the original stays available for request.error(msg) when a Rejected bubbles up.  

Handlers rebuilt to if let Err(msg) = ...{ return Ok(Rejected(msg)); } + Ok(DispatchOutcome::Continue) endings; and_then chains in the execution handlers map to DispatchOutcome::Continue, and respond_error is gone from the happy path.
A bunch of reject_after_termination calls sat before a state check that already excludes Terminated, so the reject was dead.  
Dropped those.

check_configuration_done_request and check_step_request collapse to their actual predicate -- require_state(Launched) on the first, require_stopped + require_thread_id on the second.  

The"configurationDone may only be sent once" arm is gone since require_state(Launched) already rejects Stopped.

Updated dap_rejects_repeated_configuration_done.stdout to the new "configurationDone requires launch" message.
Dropped DispatchOutcome::Rejected in favor of HandlerError, which has
Reject and Transport variants.  Predicates stay Result<(), &str>;
callers do .map_err(HandlerError::Reject)?.

With From<ServerError> for HandlerError, self.server.respond(..)? in
handlers just works.  run_requests now sends request.error(msg) for
rejections and bubbles transport errors out — one send per request.

This addresses the feedback about Result<bool, E> and predicates
eagerly reporting inside the require methods.
require_thread_id now takes i64.  Callers already know which command
they are handling, so they pull thread_id directly.  
This was the last predicate that took &Request.

Inlined require_initialized at its one callsite, single matches! check, no point keeping it separate.

The dispatch extraction arms use bug!("wrong command") for the impossible fallback, matching the existing bug! style in the file.
Handlers no longer take Request or call request.success/error themselves; they return HandlerSuccess { response, state, events, outcome } and run_requests is the single send site for both success and error responses, applying state transitions and forwarding events in emitted order.

Drop respond_terminated and respond_execution_error -- their response/event construction moves inline at the ExecutionOutcome match arms. send_stopped_event becomes stopped_event_body (pure).

dispatch_request takes &Request instead of owning+cloning; handlers receive already-destructured args.  handle_unsupported_request takes &Command. HandlerError is gone; handlers return Result<HandlerSuccess, &'static str> so predicate errors bubble via plain ?.

Note: state mutations now happen after the response send, not before. If a transport write fails, state is left untouched rather than half-mutated.  Wire output is unchanged for the success path.
@moabo3li
moabo3li force-pushed the priroda-dap-startup branch from e301817 to ffa03ae Compare August 5, 2026 17:36
@moabo3li

moabo3li commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

rebased the branch to address the review feedback in smaller commits, the big "address DAP review" commit got split into four (interpreter error rendering via to_string, exhaustive Command dispatch, routing request arguments into handlers, and the Locals scope source fill) and the dead-code removal got split into the dispatch return-type rework and the actual guard cleanup

the Result<bool, E> shape is fully gone now, predicates are Result<(), &'static str>, handlers return Result<HandlerSuccess, &'static str> with no Request, and run_requests is the single success + error send site, state transitions and event forwarding happen after the response send so a transport failure leaves session state untouched, wire output is unchanged except the repeated configurationDone fixture message text

@oli-obk oli-obk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok 😅 we're entering code golf territory

lgtm now, we can iterate more when you add more things, as those may affect further design

View changes since this review

session: &mut PrirodaContext<'tcx>,
) -> Result<(), ServerError> {
loop {
let request = match self.server.poll_request() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
let request = match self.server.poll_request() {
let request = match self.server.poll_request()? {

@oli-obk
oli-obk added this pull request to the merge queue Aug 5, 2026
Merged via the queue into rust-lang:master with commit 9b8609f Aug 5, 2026
14 checks passed
@rustbot rustbot removed the S-waiting-on-author Status: Waiting for the PR author to address review comments label Aug 5, 2026
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.

3 participants