diff --git a/NEXT.md b/NEXT.md index f6ce6b0..9917ea5 100644 --- a/NEXT.md +++ b/NEXT.md @@ -11,10 +11,14 @@ running process. Conditions are two steps in of four. it, so only a transfer gets past; with nothing transferring the program stops and names the condition. `flan_error` is where the break loop goes. -**The break loop is in.** An unhandled `error` now stops the program on the frame that erred, lists the restarts on -offer, and waits; a choice sent to the agent's socket resumes it. See "The break loop" below. What is left of §2 is the -*editor* half — a `break` notification on the daemon protocol and a minibuffer prompt — which is what makes it something -you use rather than something you drive with a socket. +**The break loop is in, editor half included.** An unhandled `error` stops the program on the frame that erred, the +daemon annotates every reply with `:stopped`/`:condition`, and `C-c C-b` in Emacs lists the restarts and resumes into +the choice. See "The break loop" and "The break loop in the editor" below — both describe what is built. + +**A restart is chosen by position now, not by name**, which is the fix for the shadowing bug and the reason `:restarts` +is a positional list with a `:unreachable` set beside it. Restarts below the evaluation a break is inside are listed, +marked, and refused with the reason; the list itself is a snapshot taken when the break was entered, because the +stopped thread's stack does not hold still. `spec-conditions.md` §4 now says all of this. **The old next task, now done, was the dev-build break loop, `spec-conditions.md` §2** — where an unhandled `error` stops and talks to the daemon instead of `rt_die()`, and where **"a crash kills the program"** finally gets fixed. The @@ -477,25 +481,31 @@ vague intention — if it is listed, someone has already established it is real. ### Bugs found and not yet fixed -- **A shadowed restart is offered and cannot be taken.** §4 says lookup takes the *first* frame offering a name, and - `flan_find_restart` does exactly that — so when two frames offer `retry`, the second one is real, is on the list the - break loop prints, and is unreachable. The old `C-c C-b` prompt showed `retry` twice and sent the string either way; - the inner frame took it, silently. `test/programs/restarts.flan`'s own `nested` function is the counterexample, and - it has been there since the transfer landed. The C&R buffer now draws the shadowed row unbracketed and refuses - `RET` on it by name, which stops the lie but does not restore the choice. The fix is to take a restart **by index** - rather than by name — SBCL identifies them positionally for precisely this reason: - - `(:op "restart-at" :index N)` on the daemon, and a matching agent verb. `flan_restart_name(i, &len)` already - walks the stack by index, so resuming by index is that loop plus `flan_break_resume`'s tail. C only, no compiler - work. +- ~~A shadowed restart is offered and cannot be taken.~~ **Fixed.** A restart is taken by *position* now: + `(:op "restart-at" :index N :name NAME)` on the daemon, `restart-at N NAME` on the agent, and a numbered + `completing-read` in `C-c C-b`. `:name` is a receipt, not the lookup — it is checked against the name the snapshot + holds at that index and refused if the two have drifted, so a bare integer can be wrong out loud. `restart ` + survives for a raw socket and is now defined as `restart-at` on the first index offering the name, so the two verbs + cannot disagree. `break.flan` grew the shadowed pair and asserts 900, which is the only value in that file no by-name + lookup can produce. The C&R buffer still marks the shadowed row by name and could now offer it instead — small, and + not done here. + +- ~~A restart chosen at a break inside a thunk is accepted, announced, and silently not taken.~~ **Fixed by refusing + it, with the reason.** Not by the depth NEXT.md proposed: recording the restart-stack depth on *entering the break + loop* counts the frames a `restart-case` inside the thunk pushed before it erred, and those are above the boundary + and work. The boundary is where it is made — `restart_floor` is set to `flan_restart_count()` around `j.call()` in + `flan_agent_poll`, saved and restored so thunks nest — and the outermost `floor` entries of the snapshot are marked + unreachable. They are listed and marked rather than hidden, refused by the listener before the reply, and carried to + the editor as `:unreachable (2 3)`. `test_dev.ml` breaks a stopped program a second time from inside `C-x C-e` and + asserts both halves: index 2 refused, index 0 taken. + +- ~~Restart names are served from a stack that is being mutated.~~ **Fixed, and it was a precondition rather than a + separate bug.** Index-based resume is wrong by construction against a moving stack: unlike a name, an index carries + no evidence of what it meant. The agent copies the list on entering `break_loop` — names into its own buffer, frames + as the addresses a transfer carries — one snapshot per nested break, and every verb answers from it. Caps are + `SNAP_MAX` 64 restarts and `SNAP_NAMES` 4096 bytes; past either, the listing says how many it did not show. Neither + cap has a test, same blind spot as the 4K result cap below. -- **A restart chosen at a break inside a thunk is accepted, announced, and silently not taken.** Demonstrated. - `flan_reload_call` allocates its own `xfer` and discards it on return, so a transfer aimed at a frame below the - `flan_agent_poll` C frame unwinds only as far as the thunk. Three statements that the program will resume, none - true. Fix: record the restart-stack depth on entering `break_loop` and refuse any frame below it. -- **Restart names are served from a stack that is being mutated.** The stopped thread is not holding still — the break - loop runs `flan_agent_poll`, which runs arbitrary Flan, and every `restart-case` it enters pushes and pops the same - global list. `flan_restart_name` can return a pointer into a popped frame, which `send` then reads out of bounds. - Fix: publish an immutable snapshot when the break loop is entered. - **The job ring has no fullness check**, and the comment describing its overflow is wrong. `publish` never consults `tail`; past `QUEUE` entries it overwrites the slot the consumer is reading, and `job` is 24 non-atomic bytes. Reachable from a program that goes a long time between `agent/poll` calls. @@ -1211,9 +1221,12 @@ a break loop has to *show* someone their choices and nothing at run time can tur there. Accepting it and discovering on the game thread that no frame offers it would answer `ok` for something that cannot happen. -The socket verbs are `restarts`, `restart ` and `abort`, and all three are refused with the reason when the -program is not stopped — there is no restart stack to walk from a running one. `test/programs/break.flan` errors twice -and the test takes a *different* restart each time, so a loop that always resumed the same way could not pass. +The socket verbs are `restarts`, `restart-at [name]`, `restart ` and `abort`, and all four are refused with +the reason when the program is not stopped — there is no restart stack to walk from a running one. `restarts` answers +` <+|-> ` per line: the index is the identity, and the flag says whether a transfer to that frame has +anywhere to land. `test/programs/break.flan` errors three times — two restarts taken by name, then a shadowed pair +where only an index can reach the outer one — so neither a loop that always resumed the same way nor one that resolved +by name could pass. ### The break loop in the editor — conditions step 3, the other half @@ -1238,13 +1251,20 @@ Three ops, and the *annotation* owns `:stopped`, not the ops — one place in th stopped, so the poll and the prompt cannot disagree. ``` -(:op "break") → (:status "ok" :restarts ("retry" …) :stopped t :condition "Missing") -(:op "restart" :name "retry") → (:status "ok" :restart "retry" :note "accepted; …") -(:op "abort") → (:status "ok" :note "the program is exiting; …") +(:op "break") → (:status "ok" :restarts ("retry" …) :unreachable (2 3) + :stopped t :condition "Missing") +(:op "restart-at" :index 2 :name "retry") → (:status "ok" :index 2 :note "accepted; …") +(:op "restart" :name "retry") → (:status "ok" :restart "retry" :note "accepted; …") +(:op "abort") → (:status "ok" :note "the program is exiting; …") ``` -`break` carries only the restart names, because those cost a second round trip to the program and are wanted only by -someone about to choose one. +`:restarts` is *positional* — innermost first, duplicates kept — because the position is what `restart-at` takes. +`:unreachable` names the positions that are on the list and cannot be chosen. `restart-at`'s `:name` is optional and is +not the lookup: the program checks it against the name it holds at that index and refuses if they have drifted, so a +prompt cannot take a different restart than the one it showed. + +`break` carries only the restart list, because it costs a second round trip to the program and is wanted only by +someone about to choose from it. **`ok` from `restart` means accepted, not resumed.** The choice is validated on the program's listener thread against the stopped stack, then taken when that thread next comes round its loop. A client that read it as "running again" would diff --git a/conditions.org b/conditions.org index 0300423..7d81d3e 100644 --- a/conditions.org +++ b/conditions.org @@ -31,10 +31,30 @@ Why it is shaped this way: [[file:spec-conditions.md][spec-conditions.md]]. Some ~defer~ between the invoke and the target runs, innermost first, before the clause body. ~errdefer~ does not. +* The break loop + +An unhandled ~error~ in a dev build stops on the frame that erred, with nothing +unwound, and waits. ~C-c C-b~ in Emacs lists what is on offer and resumes into +the choice; ~flan:stopped(Missing)~ in the modeline says it happened. + +The list is *numbered*, and the number is what is chosen. Two frames offering +~retry~ both appear and §4's by-name walk can only ever reach the first, so a +name cannot say which one is meant — ~restart-at~ can. + +A restart below the evaluation a break is inside is listed, marked, and +refused: ~C-x C-e~ runs its thunk through a C frame that holds its own transfer +channel, so an unwind aimed past it would stop at the thunk. Choose one offered +above it, or ~abort~. + * Not yet ~handler-case~ · ~find-restart~ · ~compute-restarts~ · restarts with -parameters · the dev-build break loop. Each refused by name with its reason. +parameters. Each refused by name with its reason. + +~find-restart~ and ~compute-restarts~ are blocked on a type rather than on +effort: §4 gives them ~(Option Restart)~ and a list, and there is no ~Restart~ +type and no list to return one in. The break loop reads the same stack through +the agent's socket instead. * Gotchas diff --git a/spec-conditions.md b/spec-conditions.md index 18c3d9b..5aedd48 100644 --- a/spec-conditions.md +++ b/spec-conditions.md @@ -63,6 +63,17 @@ null check. implementation, because restarts are dynamically scoped and named. A statically tracked restart set (Zig's error-set model) remains a nice-to-have. +**Open: a clause should carry a report string.** `use-placeholder` is an +identifier, which is what `invoke-restart` needs and not what a person reading a +break loop's list needs — "carry on with a blank asset" is. SBCL's restart +struct has a `report-function` for exactly this prompt, and an +`interactive-function` for the parameters §3 already has. Nothing here mentions +either, and the break loop today shows names because names are all there are. +The cost is a string constant per clause, a field beside the name in the restart +frame, and one accessor: it is not hard, it is simply not written. It should be +settled before restarts with parameters, which is the feature that makes a bare +name least sufficient. + ## 4. Name shadowing Restart lookup walks the dynamic restart stack from innermost outward and takes @@ -74,6 +85,24 @@ found before an outer one's. `(find-restart 'name)` returns `(Option Restart)` so a handler can test before committing; `(compute-restarts)` lists the visible frames for the debugger. +**A debugger identifies a restart by its position, not by its name.** The rule +above is what a handler wants — an inner `skip-form` should win — and it is +exactly wrong for a human being shown a list: a shadowed frame is on that list +and by name is unreachable, so offering it and resolving by name means taking a +different restart than the one that was pointed at. So the break loop numbers +its list, innermost first, and a choice is a position. `invoke-restart` is +unchanged and stays by name. This is why SBCL's debugger is positional too. + +A position only means something against a stack that is holding still, which +the stopped thread's is not — the break loop runs evaluations, and each one +pushes and pops this list. The list a debugger shows is therefore a **snapshot** +taken when the break was entered, and the positions are positions in it. + +**Not every visible restart is reachable.** Transfer is lowered explicitly (§6), +so it cannot cross a frame that does not carry the channel. An evaluation run +into a stopped program is called through such a frame, and a restart below it +must be refused with the reason rather than accepted and dropped. + ## 5. Cleanup during a transfer Invoking a restart transfers control outward past zero or more frames.