Say in the specs what the break loop actually does now
spec-conditions.md §4 gains the rule the shadowing bug was hiding: a handler matches by name, a debugger identifies by position, and the two are not the same question. With it, the snapshot — a position means nothing against a stack that moves — and the fact that a visible restart may still be unreachable, which §6's explicit lowering makes possible. §3 records the open one: a clause should carry a report string. is what invoke-restart needs and not what a person reading a list needs. It wants settling before restarts with parameters, which is where a bare name is least sufficient. conditions.org had the break loop under "Not yet", which it has not been for some time, and now says why find-restart and compute-restarts still are: they are blocked on a Restart type and a list to return one in, not on effort.
This commit is contained in:
parent
ac7d4a0e95
commit
91d1368279
80
NEXT.md
80
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 <name>`
|
||||
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 <name>` 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 <n> [name]`, `restart <name>` 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
|
||||
`<index> <+|-> <name>` 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
|
||||
|
||||
@ -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
|
||||
|
||||
|
||||
@ -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.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user