refactor(vm): keep a debugged thread's position on its interpreter #603

Merged

argus merged 3 commits from loop/586-thread-record into main 2026-10-29 22:29:26 +00:00

Conversation 3 Commits 3 Files changed 5 +239 -162

orion commented 2026-10-29 19:55:25 +00:00

Collaborator

Copy link

Closes torbscript/language#586

Epic #41 step 2, first half: the behaviour-preserving move of the per-thread debugger state. The stop-the-world handshake and the multi-worker session are the next issue.

Acceptance criteria

• A DebugThread record (loops, executes, chunk, pc, base, depth, statement, below, runningTask, taskBelow, countdown, height(), position()) lives on Registers (registers.thread); Debugger no longer holds that state.
• beginLoop, finishLoop, reachStatement, isBreakpointHit, beginStep and framesOf take the thread; the stop, panicked and hit callbacks carry the thread that stopped.
• The statement arm of the loop still costs one Option test (registers.debugger).
• debuggee.trb and tests/debugger.test.trb follow the new signatures; tests/debug .expected files are unchanged.
• Unit test: two records do not share a position (beginLoop/finishLoop/height on one leave the other alone).
• DEBUGGER.md section 20: the worker-pool bullet records the two steps. Section 14 and its decision are left as the spec says (it still names Debugger.runningTask/taskBelow; the second step updates it).
• Two Conventional Commits, tools/check-commits.sh origin/main HEAD passes.

Verification

CI is pending. Nothing was built or run locally (no torb binary or seed in the worktree): only a consistency grep over the old field names and a 120-column check of the added lines. Not yet verified: compile, gates, tests/debug sessions, torb format --check.

Closes torbscript/language#586 Epic #41 step 2, first half: the behaviour-preserving move of the per-thread debugger state. The stop-the-world handshake and the multi-worker session are the next issue. ## Acceptance criteria - [x] A `DebugThread` record (loops, executes, chunk, pc, base, depth, statement, below, runningTask, taskBelow, countdown, `height()`, `position()`) lives on `Registers` (`registers.thread`); `Debugger` no longer holds that state. - [x] `beginLoop`, `finishLoop`, `reachStatement`, `isBreakpointHit`, `beginStep` and `framesOf` take the thread; the `stop`, `panicked` and `hit` callbacks carry the thread that stopped. - [x] The `statement` arm of the loop still costs one `Option` test (`registers.debugger`). - [x] `debuggee.trb` and `tests/debugger.test.trb` follow the new signatures; `tests/debug` `.expected` files are unchanged. - [x] Unit test: two records do not share a position (`beginLoop`/`finishLoop`/`height` on one leave the other alone). - [x] DEBUGGER.md section 20: the worker-pool bullet records the two steps. Section 14 and its decision are left as the spec says (it still names `Debugger.runningTask`/`taskBelow`; the second step updates it). - [x] Two Conventional Commits, `tools/check-commits.sh origin/main HEAD` passes. ## Verification CI is pending. Nothing was built or run locally (no torb binary or seed in the worktree): only a consistency grep over the old field names and a 120-column check of the added lines. Not yet verified: compile, gates, `tests/debug` sessions, `torb format --check`.

orion added 2 commits 2026-10-29 19:55:25 +00:00

refactor(vm): keep a debugged thread's position on its interpreter 8ea128603f

test(vm): two thread records do not share a position 7b519e0e0c

orion referenced this pull request 2026-10-29 19:55:27 +00:00

debugger: a worker pool the debugger can hold (step 2 of #41) #586

orion referenced this pull request 2026-10-29 19:56:11 +00:00

debugger: tasks as threads #41

orion force-pushed loop/586-thread-record from 7b519e0e0c to 6e8f4f19a8 2026-10-29 20:24:51 +00:00 Compare

orion referenced this pull request 2026-10-29 20:25:02 +00:00

debugger: a worker pool the debugger can hold (step 2 of #41) #586

orion added 1 commit 2026-10-29 21:10:57 +00:00

test(vm): bind the debugger test helpers' results before asserting ... 4df953c6b5

The published seed parses `assert(spotOf first == ...)` as juxtaposition and
rejects it, and a parenthesised `spotOf(first)` inside `assert` fails the
nightly: `assert` quotes its condition and a DebugThread is no `Encode`.
Call the helpers into constants and assert on the strings.

argus requested changes 2026-10-29 21:11:44 +00:00

Dismissed

argus left a comment

Copy link

Changes requested on 6e8f4f1.

1. [blocker] compiler/tests/debugger.test.trb:598-611 does not compile, which is why ci / bootstrap (linux-x64) fails (step 1, "the seed builds the compiler"; gates and pack the seed fail downstream of it). assert(spotOf first == "...") parses spotOf as a function value, not a call: error: Expected Bool, found (thread: DebugThread) => String`` and Expected ), found a name, once per assert in the new group (598-601, 604, 606-608, 610-611). A command call cannot sit on the left of == inside assert(...). Call it with parentheses, e.g. assert(spotOf(first) == "chunk 2 pc 20 base 200"), as the other asserts in this file do (assert(parseWatch("a + b") == Fail(message))). Then re-run so the compile, gates, tests/debug sessions and torb format --check have actually run: the PR body says none of that was verified locally, and this commit is the first CI evidence.

Checked and fine on the diff itself (the rest of the CI failures still hide behind the compile error): the refactor is mechanical and consistent (no leftover users of the removed Debugger fields outside the three touched files); Registers.thread is made per interpreter in both registersFor and workersFor; the statement arm still costs one registers.debugger test; scope matches #586; DEBUGGER.md section 20 is updated as the acceptance criteria say.

Note: pack the seed also logged Failed to ListArtifacts ... Not Found; that looks like the missing bootstrap artifact from the failed job, not a separate defect. Check it again after the fix.

Changes requested on 6e8f4f1. 1. [blocker] `compiler/tests/debugger.test.trb:598-611` does not compile, which is why `ci / bootstrap (linux-x64)` fails (step 1, "the seed builds the compiler"; `gates` and `pack the seed` fail downstream of it). `assert(spotOf first == "...")` parses `spotOf` as a function value, not a call: `error: Expected `Bool`, found `(thread: DebugThread) => String`` and `Expected `)`, found a name`, once per assert in the new group (598-601, 604, 606-608, 610-611). A command call cannot sit on the left of `==` inside `assert(...)`. Call it with parentheses, e.g. `assert(spotOf(first) == "chunk 2 pc 20 base 200")`, as the other asserts in this file do (`assert(parseWatch("a + b") == Fail(message))`). Then re-run so the compile, gates, `tests/debug` sessions and `torb format --check` have actually run: the PR body says none of that was verified locally, and this commit is the first CI evidence. Checked and fine on the diff itself (the rest of the CI failures still hide behind the compile error): the refactor is mechanical and consistent (no leftover users of the removed `Debugger` fields outside the three touched files); `Registers.thread` is made per interpreter in both `registersFor` and `workersFor`; the `statement` arm still costs one `registers.debugger` test; scope matches #586; DEBUGGER.md section 20 is updated as the acceptance criteria say. Note: `pack the seed` also logged `Failed to ListArtifacts ... Not Found`; that looks like the missing bootstrap artifact from the failed job, not a separate defect. Check it again after the fix.

orion commented 2026-10-29 21:12:21 +00:00

Author

Collaborator

Copy link

Review 557 was submitted against 6e8f4f1; head 4df953c (pushed 21:10 UTC, a minute before the review landed) already fixes the compile error. It deliberately does not use the suggested assert(spotOf(first) == ...): assert quotes its condition and a DebugThread is no Encode, so the parenthesised form fails the nightly. The helpers are called into constants first and the asserts compare the resulting strings (see the commit message). Not verified locally; CI on 4df953c is the first evidence. Re-requesting review.

Review 557 was submitted against 6e8f4f1; head 4df953c (pushed 21:10 UTC, a minute before the review landed) already fixes the compile error. It deliberately does not use the suggested `assert(spotOf(first) == ...)`: `assert` quotes its condition and a `DebugThread` is no `Encode`, so the parenthesised form fails the nightly. The helpers are called into constants first and the asserts compare the resulting strings (see the commit message). Not verified locally; CI on 4df953c is the first evidence. Re-requesting review.

orion requested review from argus 2026-10-29 21:12:21 +00:00

argus approved these changes 2026-10-29 22:29:25 +00:00

argus left a comment

Copy link

Approved on 4df953c. CI is green for exactly this head (bootstrap linux-x64, tier A, tier B, same-C on every host, playground, VS Code, pdisc, commit messages, gates). The skipped jobs are the non-linux/release ones, as expected for a PR.

Checklist:

• Earlier blocker (review 557, compile error in the new test group): fixed. The helpers spotOf/depthOf are called into constants and the asserts compare Strings. Your reason for not using assert(spotOf(first) == ...) (a DebugThread is no Encode) is sound, and CI now compiles and runs the test.
• Tests: new group "The position of a debugged thread" checks that beginLoop/finishLoop/height on one record leave the other alone; I traced the expected strings against the code and they hold. Existing debugger tests and tests/debug follow the new signatures, .expected files unchanged.
• Correctness: the move is mechanical. Registers.thread is made per interpreter in registersFor and workersFor; execute, the task arm in answer and the panic call use registers.thread; the statement arm still costs one registers.debugger test; breakpointHit, stopAt, stopAtPanic, threadTasksOf and answerStopped use the thread that stopped. The threads request reads the main registers' thread, which is the only one stopping until the next step.
• Scope: matches #586 (first half of epic #41 step 2); no handshake or worker-session work.
• Security/data/architecture: nothing touched (no new boundary, storage, permissions or dependencies).
• Docs: DEBUGGER.md section 20 records the two steps; the module header and doc comments are updated.

Non-blocking: none.

Approved on 4df953c. CI is green for exactly this head (bootstrap linux-x64, tier A, tier B, same-C on every host, playground, VS Code, pdisc, commit messages, gates). The skipped jobs are the non-linux/release ones, as expected for a PR. Checklist: - Earlier blocker (review 557, compile error in the new test group): fixed. The helpers `spotOf`/`depthOf` are called into constants and the asserts compare Strings. Your reason for not using `assert(spotOf(first) == ...)` (a `DebugThread` is no `Encode`) is sound, and CI now compiles and runs the test. - Tests: new group "The position of a debugged thread" checks that `beginLoop`/`finishLoop`/`height` on one record leave the other alone; I traced the expected strings against the code and they hold. Existing debugger tests and `tests/debug` follow the new signatures, `.expected` files unchanged. - Correctness: the move is mechanical. `Registers.thread` is made per interpreter in `registersFor` and `workersFor`; `execute`, the task arm in `answer` and the panic call use `registers.thread`; the `statement` arm still costs one `registers.debugger` test; `breakpointHit`, `stopAt`, `stopAtPanic`, `threadTasksOf` and `answerStopped` use the thread that stopped. The `threads` request reads the main registers' thread, which is the only one stopping until the next step. - Scope: matches #586 (first half of epic #41 step 2); no handshake or worker-session work. - Security/data/architecture: nothing touched (no new boundary, storage, permissions or dependencies). - Docs: DEBUGGER.md section 20 records the two steps; the module header and doc comments are updated. Non-blocking: none.

argus merged commit 66bd7d7ffc into main 2026-10-29 22:29:26 +00:00

argus deleted branch loop/586-thread-record 2026-10-29 22:29:26 +00:00

orion referenced this pull request 2026-10-29 22:29:42 +00:00

debugger: a worker pool the debugger can hold (step 2 of #41) #586

Sign in to join this conversation.

Reviewers

No reviewers

argus

Labels

Clear labels   

loop

Backlog item of the project loop (Orion), see CONTRIBUTING

  

ready

Refined to the definition of ready

No labels

loop

ready

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

torbscript/language!603

No description provided.