diff --git a/NEXT.md b/NEXT.md index 38170bf..b7beb71 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 @@ -36,16 +40,20 @@ which §6 rules out. ### In flight — four tracks, one session, 2026-09-12 -Four agents are working these in parallel worktrees. Listed so a session reading this cold does not start one of them again. +Four agents, parallel worktrees. Listed so a session reading this cold does not start one of them again. 1 and 2 are +merged; 3 and 4 are still running. -1. **`nth` removed, then a `println` builtin.** `nth` was an *asymmetric* alias of `at`: `check.ml` aliased them, but +1. **Done.** `nth` removed, and `println` built. `nth` was an *asymmetric* alias of `at`: `check.ml` aliased them, but `parse.ml` and `place_of_expr` matched only `at`, so `(set (nth a i) x)` and `(addr (nth a i))` were refused while - the `at` forms worked. Two names documented as identical that differ on write. Then `println`: `Session.render` is - already the compile-time structural walk plan.org asks for, and lifting it out of the dev path — parameterised on - its emitter — gives a real `println` over concrete types with none of milestone 5 involved. The per-type - `print-f64`/`print-str` family goes. -2. **The break loop's editor half.** `break` on the daemon protocol, `compute-restarts`, `restart-at`, and a - minibuffer prompt. `spec-conditions.md` §2's remaining piece. + the `at` forms worked. `println` lifted `Session.render` into `lib/render.ml`, parameterised on an emitter and a + slot allocator, so the REPL and stdout share one walk. Found on the way: `field_addr` in `emit.ml` accepted only + `Types.Named`, so a field of an `Option` threw at emit time and **the walk's Option arm had never run** — the + inspector would have failed on the first `(Option T)` pointed at it. +2. **Done.** `restart-at`, by position. The editor half already existed — this session's brief was written off a + stale "Start here" line and is corrected below. The real work was taking a restart the innermost frame shadows: + `restart ` resolves to the inner one, so a visible outer restart could not be taken at all. An immutable + snapshot per break makes an index mean something, `:name` rides along as a receipt checked against it, and a + generation stamp keeps a nested break from claiming the outer one's choice. 3. **The first ten raylib core examples**, plus the window and input bindings they need. The deliverable is the *language gap list* as much as the ported files — sand.flan is the only real raylib program today, so this is the first time the language is pushed by code it was not designed around. @@ -55,6 +63,24 @@ Four agents are working these in parallel worktrees. Listed so a session reading `Emit.redefinition` already takes and is already tested for. The second also unblocks source interleaving in the disassembly buffer. +### Queued — the `print-*` family goes + +`print` and `println` are the whole printing surface. `print-str`, `print-i64`, `print-f64`, `print-bytes`, +`print-line` and `newline` are kept only until tracks 3 and 4 land, because an agent working in a tree that still has +them will write new fixtures that use them and the sweep would run twice. + +`print` subsumes all of them, and is *better*: there is no implicit widening here, so `print-i64` forces an explicit +`(i64 x)` at every call site and `print` takes the `i32` as it is. `(print-line s)` is `(println s)`; `(newline)` is +`(println "")`. + +About 470 call sites across ~60 files. Two places want reading rather than `sed`: + +- `(print-i64 (i64 x))` where `x` is a `u64` changes its **answer**, not just its spelling — the old form widened + through the signed printer and printed `-1` for `0xFFFF…F`, and `print` routes a `u64` through its own shim. The + pinned `.out` files move, and each move is a thing to look at. +- `web/index.html` (55 sites) and `web/examples/` carry pinned output, so the prose changes with them. That is also + the moment to document `println` at all: it currently has **no mention on the language's own doc page**. + **DWARF is not missing, and `flan-cnr.el`'s stack pane reads as though it were.** `flan build --debug` emits it and `flan-dape.el` drives lldb with it. What the break loop lacks is DWARF *in a dev build* — a `flan dev` process is not a `--debug` one. Worth rewording that refusal so it names the real gap. @@ -502,25 +528,41 @@ 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 snapshot generation has no test, and the window is a race.** A choice is validated against the snapshot on top + when the request arrives and resolved against the snapshot on top when the game thread next looks. Between those, + an evaluation the break loop is running can error and push a break of its own, whose loop would otherwise reach + [chosen_ready] first and take *its* index 2 for the one someone chose from the outer list. Each snapshot now carries + a generation, a choice is stamped with the one it was validated against, and a loop claims only what is addressed to + it — a mismatch is left set rather than discarded, because the listener already answered ok for it. Depth would not + do: an outer break resuming and a new one starting reuses the number. None of this is tested, because arranging the + window means landing a request inside a two-millisecond poll from outside the process. It wants a hook the test can + drive, not a sleep. -- **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. @@ -1236,9 +1278,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 @@ -1263,13 +1308,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/emacs/flan-dev.el b/emacs/flan-dev.el index fc612b6..a1e8c03 100644 --- a/emacs/flan-dev.el +++ b/emacs/flan-dev.el @@ -609,12 +609,51 @@ every bit as connected and nothing like running." ;; first. `completing-read' over them is the natural shape: they are a closed ;; set the program computed, so require-match is exactly right. ;; +;; What is chosen is a *position* on that list, not a name. §4 says lookup +;; takes the first frame offering a name, so when two frames offer `retry' the +;; outer one is real, is on this list, and by name is unreachable — the old +;; prompt showed `retry' twice and sent the string either way, and the inner +;; frame took it silently. A position cannot be ambiguous, which is why SBCL +;; identifies restarts positionally too. So the candidates are numbered and +;; the number is what goes on the wire. +;; ;; `abort' is on the same list rather than on a separate key, because it is the ;; same decision: it is what you pick when none of the restarts is the answer. ;; It is last, and it is not the default. +(defun flan-dev--restart-candidates (restarts unreachable) + "Label each of RESTARTS by its position, marking those in UNREACHABLE. +An alist of label to index. The index leads the label because it is the +identity: two entries may read the same and mean different frames." + (let ((i -1)) + (mapcar (lambda (name) + (setq i (1+ i)) + (cons (format "%d. %s%s" i name + (if (memq i unreachable) + " (below this break; cannot be taken)" + "")) + i)) + restarts))) + +(defun flan-dev-restart-at (index name) + "Resume the stopped program at the restart at position INDEX. +NAME is sent with it and is not the lookup: the program checks it against +the name it holds at that position and refuses if the two have drifted +apart, so a prompt cannot take a different restart than the one it showed." + (let ((r (flan-dev--request (list :op "restart-at" :index index :name name)))) + (if (equal (plist-get r :status) "ok") + (progn + ;; Accepted, not resumed — see `flan-dev-restart'. + (setq flan-dev--stopped nil) + (force-mode-line-update t) + (message "flan: %s — %s" name (or (plist-get r :note) "accepted"))) + (user-error "flan: %s" (or (plist-get r :message) "refused"))))) + (defun flan-dev-restart (name) - "Resume the stopped program at the restart called NAME." + "Resume the stopped program at the restart called NAME. +The first frame offering NAME, which is §4's own rule and therefore cannot +reach a shadowed one. `flan-break' chooses by position instead; this is +here for a name known in advance." (interactive (list (completing-read "Restart: " (flan-dev-restarts) nil t))) (let ((r (flan-dev--request (list :op "restart" :name name)))) (if (equal (plist-get r :status) "ok") @@ -664,12 +703,21 @@ than being told so." (unless flan-dev--stopped (user-error "flan: the program is running; nothing is stopped")) (let* ((restarts (plist-get r :restarts)) + (unreachable (append (plist-get r :unreachable) nil)) + (table (flan-dev--restart-candidates restarts unreachable)) (choice (completing-read (format "flan: stopped on %s%s — " flan-dev--stopped (if restarts "" " (no restarts are active)")) - (append restarts '("abort")) nil t))) - (if (equal choice "abort") (flan-dev-abort) (flan-dev-restart choice))))) + (append (mapcar #'car table) '("abort")) nil t)) + (index (cdr (assoc choice table)))) + (cond + ((equal choice "abort") (flan-dev-abort)) + ;; `require-match' over a table this built, so a choice outside it is + ;; not something a person can type — but deriving the table wrongly + ;; should say so rather than put nil on the wire as an index. + ((null index) (user-error "flan: %s is not on the list" choice)) + (t (flan-dev-restart-at index (nth index restarts))))))) ;;;###autoload (defun flan-show-output () diff --git a/emacs/test-flan-dev.el b/emacs/test-flan-dev.el index d21a932..de6d605 100644 --- a/emacs/test-flan-dev.el +++ b/emacs/test-flan-dev.el @@ -420,6 +420,26 @@ is written instead — the real `message' call the real command makes." (test-flan--check "the restarts on offer are the ones the frame declared" (equal (flan-dev-restarts) '("use-placeholder"))) + ;; And what it would put in front of someone. The labels carry the position, + ;; because the position is what gets chosen: two frames may offer the same + ;; name and only a number can say which one. A pure function over a reply, + ;; so it is checked against the shapes a real daemon cannot easily be made to + ;; produce as well as against the one it just did. + (test-flan--check "the prompt numbers what it offers" + (equal (flan-dev--restart-candidates '("use-placeholder") nil) + '(("0. use-placeholder" . 0)))) + (test-flan--check "a shadowed name is two distinguishable choices" + (equal (mapcar #'cdr + (flan-dev--restart-candidates + '("retry" "use-placeholder" "retry") nil)) + '(0 1 2))) + (test-flan--check "a restart below the break is shown, and shown as such" + (let ((table (flan-dev--restart-candidates + '("retry" "use-placeholder") '(1)))) + (and (not (string-match-p "cannot be taken" (caar table))) + (string-match-p "cannot be taken" (car (nth 1 table))) + (equal (cdr (nth 1 table)) 1)))) + ;; The payoff. The break loop *is* the poll loop, so an expression sent now ;; runs on the stopped thread and comes back — which is the one moment ;; anybody actually wants C-x C-e to work. @@ -445,7 +465,9 @@ is written instead — the real `message' call the real command makes." ;; Choosing one. "ok" from the daemon means accepted — the stopped thread ;; takes it on its next pass — so the client stops claiming a break and lets ;; the next poll settle it. - (flan-dev-restart "use-placeholder") + ;; By position, which is the path `C-c C-b' takes: the name goes with it as + ;; the receipt the program checks, not as the lookup. + (flan-dev-restart-at 0 "use-placeholder") (let ((deadline (+ (float-time) 20))) (while (and (not (eq (flan-dev-state) 'live)) (< (float-time) deadline)) (flan-dev--poll) diff --git a/lib/dev.ml b/lib/dev.ml index 0cb1dfc..8a83abd 100644 --- a/lib/dev.ml +++ b/lib/dev.ml @@ -195,18 +195,40 @@ let state t = (* Innermost first, terminated by a line that is a single dot — the agent's framing, not this one's. A refusal comes back as a line starting "err ", and is passed on rather than turned into an empty list: no restarts and cannot - say are different answers. *) + say are different answers. + + Each line is [I ± NAME]: the index it is taken by, whether it can be taken, + and the name. The index is the identity — two frames may offer [retry] and + a name cannot say which — and it is the program's number, not this end's + position in a list, so it is carried rather than recomputed. *) let restarts t = match ask t "restarts" with | text -> let lines = String.split_on_char '\n' text in if List.exists (fun l -> String.length l >= 3 && String.sub l 0 3 = "err") lines then Error (String.trim text) - else + else begin + let parse line = + match String.index_opt line ' ' with + | None -> None + | Some i -> + (match int_of_string_opt (String.sub line 0 i) with + | None -> None + | Some idx -> + let rest = String.sub line (i + 1) (String.length line - i - 1) in + if String.length rest < 2 then None + else + Some + ( idx, + rest.[0] = '+', + String.sub rest 2 (String.length rest - 2) )) + in Ok - (List.filter - (fun l -> l <> "" && l <> ".") - (List.map String.trim lines)) + (List.filter_map parse + (List.filter + (fun l -> l <> "" && l <> ".") + (List.map String.trim lines))) + end | exception Unix.Unix_error (e, _, _) -> Error (Unix.error_message e) let alive t = @@ -465,7 +487,21 @@ let break t = | Unreachable m -> error ("cannot ask the program whether it stopped: " ^ m) | Stopped _ -> (match restarts t with - | Ok names -> ok [ ":restarts " ^ Wire.strings names ] + | Ok rs -> + (* [:restarts] stays a list of names, positional and innermost first, + with duplicates kept — the position *is* the index, which is what + [restart-at] takes. [:unreachable] names the positions that are on + the list and cannot be chosen: a restart below the evaluation the + break is inside has nowhere for a transfer to land. They are shown + rather than filtered, because a client that quietly dropped them + would leave someone asking where their restart went. *) + ok + [ ":restarts " ^ Wire.strings (List.map (fun (_, _, n) -> n) rs); + ":unreachable " + ^ Wire.ints + (List.filter_map + (fun (i, ok, _) -> if ok then None else Some i) + rs) ] | Error m -> error ("the program refused to list its restarts: " ^ m)) (* A choice is validated by the *program*, on its listener thread, against a @@ -477,6 +513,30 @@ let break t = stopped thread next comes round its loop, which is microseconds away and still not now. An editor that read [ok] as "running again" would poll once, find it stopped, and re-open the prompt it had just answered. *) +let choose_at t ~index ~name = + if not (alive t) then error "the program exited; restart flan dev" + else if + match name with + | Some n -> String.exists (fun c -> Char.code c < 32 || Char.code c = 127) n + | None -> false + then error "a restart name cannot contain a control character" + else + let verb = + "restart-at " ^ string_of_int index + ^ match name with Some n -> " " ^ n | None -> "" + in + match ask t verb with + | reply when String.trim reply = "ok" -> + ok + [ ":index " ^ string_of_int index; + ":note " + ^ Wire.quote + "accepted; the program resumes at its next pass of the break loop" + ] + | reply -> error (String.trim reply) + | exception Unix.Unix_error (e, _, _) -> + error ("cannot reach the program: " ^ Unix.error_message e) + let choose t ~name = if not (alive t) then error "the program exited; restart flan dev" else if String.exists (fun c -> Char.code c < 32 || Char.code c = 127) name then @@ -836,6 +896,15 @@ let handle t req = (match Wire.string_field req "name" with | Some name -> choose t ~name | None -> error "restart needs :name") + (* By index, which is the one that can name a shadowed restart. [:name] is + optional and is not the lookup: it is checked against the name the program + has at that index and refused if they have drifted apart, so a client that + listed and then chose cannot take a different restart than the one it + showed. *) + | Some "restart-at" -> + (match Wire.int_field req "index" with + | Some index -> choose_at t ~index ~name:(Wire.string_field req "name") + | None -> error "restart-at needs :index") | Some "abort" -> abort t | Some "disassemble" -> (match Wire.string_field req "name" with diff --git a/lib/wire.ml b/lib/wire.ml index d2d4b23..0a1d0ad 100644 --- a/lib/wire.ml +++ b/lib/wire.ml @@ -89,6 +89,16 @@ let string_field form key = | Some { Form.v = Form.Str s; _ } -> Some s | _ -> None +(* A restart is chosen by index, so the protocol has to carry one. Kept as + narrow as [string_field]: a form that is not an integer is [None] and the + op says what it wanted, rather than this guessing at a string. *) +let int_field form key = + match field form key with + | Some { Form.v = Form.Int i; _ } -> Some (Int64.to_int i) + | _ -> None + +let ints ns = list (List.map string_of_int ns) + let parse src = match Reader.read_all ~file:"" src with | [ f ] -> f diff --git a/runtime/flan_rt.c b/runtime/flan_rt.c index 28de468..0a01a08 100644 --- a/runtime/flan_rt.c +++ b/runtime/flan_rt.c @@ -116,6 +116,29 @@ void *flan_find_restart(uint32_t name_id) { return NULL; } +/* The i'th frame itself, innermost first — the same walk as + * [flan_restart_name] and the other half of it. A break loop that offers + * someone a *list* has to be able to take the entry they picked, and §4's + * by-name lookup cannot express "the second retry": it takes the first frame + * offering the name, by definition, so a shadowed restart is on every list + * and reachable from none of them. Identifying a restart positionally is the + * only thing that fixes that, and it is why SBCL does the same. + * + * The address is the currency: a transfer carries the frame's address, so a + * caller that holds one from before is holding the same frame the walk found, + * whatever the walk finds today. */ +void *flan_restart_frame(int32_t i) { + for (flan_restart *r = restarts; r != NULL; r = r->prev) + if (i-- == 0) return r; + return NULL; +} + +/* Aim the transfer channel at a frame obtained earlier. The same store + * [flan_break_resume] makes and the same one an invoke-restart makes — this + * only spells it without a lookup, for a caller that did its looking up when + * the stack was worth reading. */ +void flan_restart_take(void *frame, void *xfer) { *(void **)xfer = frame; } + /* [T] and string are both ptr+len — see Emit.ll. */ typedef struct { const uint8_t *ptr; int64_t len; } flan_slice; 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. diff --git a/test/programs/break.flan b/test/programs/break.flan index d56ca52..12bc656 100644 --- a/test/programs/break.flan +++ b/test/programs/break.flan @@ -14,8 +14,21 @@ (use-placeholder [] -1) (retry [] 7))) +;;; Two frames offering the same name, which §4 says resolves to the inner one +;;; and only ever the inner one. The outer clause is therefore on every list a +;;; break loop prints and reachable from no name at all, which is why a restart +;;; is taken by *index* now. 900 is the proof: it is the only value in this +;;; file that by-name lookup cannot produce. +(defn shadowed [n i32] i32 + (restart-case + (+ (restart-case (do (error (Missing {:id n})) 0) + (retry [] 5)) + 100) + (retry [] 900))) + (defn main [] i32 (agent/start "/tmp/flan-break.sock") (print-i64 (i64 (fetch 1))) (newline) (print-i64 (i64 (fetch 2))) (newline) + (print-i64 (i64 (shadowed 3))) (newline) 0) diff --git a/test/test_agent.ml b/test/test_agent.ml index 4f16c62..9627b1b 100644 --- a/test/test_agent.ml +++ b/test/test_agent.ml @@ -200,9 +200,9 @@ let () = then fail "the program never reached the break loop: %S" !listed else begin (* Innermost first, and both on offer. *) - if !listed <> "retry\nuse-placeholder\n.\n" then + if !listed <> "0 + retry\n1 + use-placeholder\n.\n" then fail "restarts on offer\n got: %S\n wanted: %S" !listed - "retry\nuse-placeholder\n.\n"; + "0 + retry\n1 + use-placeholder\n.\n"; (* A name nothing offers is refused *here*, before the reply. Answering ok and discovering it on the game thread would report success for something that cannot happen. *) @@ -223,9 +223,41 @@ let () = if not (await printed) then fail "the first restart never produced its value" else if not (await (fun () -> send bsock "restarts" - = "retry\nuse-placeholder\n.\n")) + = "0 + retry\n1 + use-placeholder\n.\n")) then fail "the program never stopped a second time" - else ignore (send bsock "restart use-placeholder") + else begin + ignore (send bsock "restart use-placeholder"); + (* -- Taking a restart by index ------------------------------- *) + + (* The third break is inside a [restart-case] whose name the frame + below it also offers. Both are listed; §4's by-name walk can only + ever reach the first. So this takes the *second*, and 900 is a + value nothing else in the program can produce — the assertion on + the output at the end is what makes this test about shadowing + rather than about a reply. *) + let printed2 () = + let t = In_channel.with_open_bin bout In_channel.input_all in + List.exists (String.equal "-1") (String.split_on_char '\n' t) + in + if not (await printed2) then + fail "the second restart never produced its value" + else if not (await (fun () -> send bsock "restarts" + = "0 + retry\n1 + retry\n.\n")) + then fail "the program never stopped on the shadowed pair" + else begin + (* Out of range is refused before the reply, like a bad name. *) + let oob = send bsock "restart-at 7" in + if not (String.length oob >= 3 && String.sub oob 0 3 = "err") then + fail "an index nothing offers was accepted: %S" oob; + (* The name rides along as a receipt, not as the lookup: an index + whose name has moved is refused rather than silently taken, + which is the same failure by-name lookup had. *) + let drift = send bsock "restart-at 1 use-placeholder" in + if not (String.length drift >= 3 && String.sub drift 0 3 = "err") + then fail "an index whose name had drifted was accepted: %S" drift; + ignore (send bsock "restart-at 1 retry") + end + end end end; let bstatus = ref (Unix.WEXITED 0) in @@ -241,7 +273,7 @@ let () = end else begin let text = In_channel.with_open_bin bout In_channel.input_all in - let want = "7\n-1\n" in + let want = "7\n-1\n900\n" in let got = String.concat "\n" (List.filter diff --git a/test/test_dev.ml b/test/test_dev.ml index 911e835..d87df25 100644 --- a/test/test_dev.ml +++ b/test/test_dev.ml @@ -296,6 +296,73 @@ let () = fail "installing while stopped: %s" (Option.value ~default:"" (Wire.string_field r "message")); + (* -- A break inside a thunk, and the frames it cannot reach ---- *) + + (* Evaluating here runs a thunk through [flan_reload_call], which holds + its own transfer channel and drops it on return. So a restart below + that C frame — the two this program was already stopped on — has + nowhere for a transfer to land: it would unwind to the thunk, stop, + and the program would carry on as though nothing had been chosen. + It used to be accepted and announced and silently not taken, which + is the worst of the three things that could happen. + + Now the list says so and the choice is refused with the reason. *) + let r = + ask "(:op \"eval-expr\" :code \"(i64 (fetch 9))\" :file \"/tmp/buf.flan\")" + in + if status r <> "error" then + fail "an expression that stopped inside a break answered anyway"; + let r = ask "(:op \"break\")" in + if status r <> "ok" then fail "break inside a thunk: %s" (status r); + (* Four now: the thunk's own [fetch] frame over the one below it. *) + let names = + match Wire.field r "restarts" with + | Some { Form.v = Form.List l; _ } -> + List.filter_map + (fun (n : Form.t) -> + match n.Form.v with Form.Str x -> Some x | _ -> None) + l + | _ -> [] + in + if names <> [ "retry"; "use-placeholder"; "retry"; "use-placeholder" ] + then fail "restarts at a break inside a thunk: %s" + (String.concat ", " names); + let unreachable = + match Wire.field r "unreachable" with + | Some { Form.v = Form.List l; _ } -> + List.filter_map + (fun (n : Form.t) -> + match n.Form.v with + | Form.Int i -> Some (Int64.to_int i) + | _ -> None) + l + | _ -> [] + in + if unreachable <> [ 2; 3 ] then + fail "positions below the thunk: %s" + (String.concat ", " (List.map string_of_int unreachable)); + (* Refused, and refused *here* — not accepted and dropped. *) + let r = ask "(:op \"restart-at\" :index 2 :name \"retry\")" in + if status r <> "error" then + fail "a restart below the thunk boundary was accepted"; + (* The ones above it still work, so this refuses a case rather than + disabling the feature. *) + let r = ask "(:op \"restart-at\" :index 0 :name \"retry\")" in + if status r <> "ok" then + fail "a restart inside the thunk was refused: %s" + (Option.value ~default:"" (Wire.string_field r "message")); + (* And the program is back on the *outer* break, which is the one it + was on before any of this — the inner resume must not have been + read as the outer one resuming. *) + if not + (await (fun () -> + let r = ask "(:op \"break\")" in + status r = "ok" + && (match Wire.field r "restarts" with + | Some { Form.v = Form.List l; _ } -> List.length l = 2 + | _ -> false))) + then fail "the outer break did not come back after the inner one"; + (* A name nothing offers is refused against the live stack, on the program's listener thread, before the reply. *) let r = ask "(:op \"restart\" :name \"nonesuch\")" in diff --git a/vendor/agent/flan_agent.c b/vendor/agent/flan_agent.c index ad749d2..eed8ad6 100644 --- a/vendor/agent/flan_agent.c +++ b/vendor/agent/flan_agent.c @@ -103,6 +103,27 @@ extern int32_t flan_break_resume(const uint8_t *name, int64_t namelen, void *xfer); extern int32_t flan_restart_count(void); extern const uint8_t *flan_restart_name(int32_t i, int64_t *len); +extern void *flan_restart_frame(int32_t i); +extern void flan_restart_take(void *frame, void *xfer); + +/* -- How far down a transfer can actually land ----------------------- */ + +/* A thunk run from this loop is called through [flan_reload_call], which + * allocates its *own* transfer channel and drops it on return. So a restart + * chosen at a break inside a thunk unwinds as far as the thunk and no + * further: every frame below that C boundary is on the list, is accepted, and + * is silently not taken. Three statements that the program will resume, none + * of them true - which is worse than either resuming or refusing. + * + * The boundary is recorded where it is *made*, at the call, not at the break: + * the frames below it are exactly the ones already on the stack when the + * thunk started, and a restart-case the thunk enters itself is above it and + * reachable. Recording the depth on entering the break loop instead would + * count those too and refuse restarts that work. + * + * Game thread only, saved and restored around the call, so a thunk that + * breaks and whose break runs another thunk nests correctly. */ +static int32_t restart_floor; /* What the listener thread hands the stopped game thread. One slot, because * only one thread is ever stopped. */ @@ -116,9 +137,100 @@ static _Atomic int depth; #define BREAK_MAX 8 /* deep enough to nest, shallow * enough that a loop of breaks * stops rather than grinds */ -static char chosen[128]; +static _Atomic int chosen_index; +/* Which snapshot that index is an index *into*. The listener validates a + * choice against the snapshot on top when the request arrives, and the game + * thread resolves it against the snapshot on top when it next looks - and + * those are two reads of a stack that can move between them. An evaluation + * this loop runs may error, push a break of its own, and reach [chosen_ready] + * first, which would take that break's index 2 for the one someone chose from + * the outer list. Depth is not enough to tell them apart: an outer break + * resuming and a new one starting reuses the number. A generation does not. + * + * A mismatch is *left alone* rather than discarded. The listener already + * answered ok for it, so the break it was meant for must still be able to + * take it; the inner loop simply does not claim what is not addressed to it. */ +static _Atomic int chosen_gen; static _Atomic int chosen_ready; static _Atomic int aborting; + +/* -- The snapshot ---------------------------------------------------- */ + +/* The stopped thread is not holding still. This loop runs [flan_agent_poll], + * which runs arbitrary Flan, and every restart-case that runs pushes and pops + * the one global restart list. Serving names straight off that list hands the + * listener a pointer into a frame that may already have been popped; serving + * *indices* off it is worse still, because an index carries no evidence of + * what it meant - the list and the choice can disagree and nothing can tell. + * + * So the list is read once, on entry, while the thread that owns it is in + * this function and not in Flan, and copied: names into a buffer of our own, + * frames as the addresses a transfer carries. Everything the listener answers + * comes from here and nothing re-reads the live stack. */ +#define SNAP_MAX 64 /* restarts offered at one break */ +#define SNAP_NAMES 4096 /* bytes of names behind them */ + +typedef struct { + int32_t gen; /* never reused, never 0 */ + int32_t n; + int32_t total; /* before SNAP_MAX truncated it */ + void *frame[SNAP_MAX]; + int32_t off[SNAP_MAX], len[SNAP_MAX]; + int32_t reachable[SNAP_MAX]; + int32_t used; + char names[SNAP_NAMES]; +} snapshot; + +/* One per nested break loop, because an inner break must not answer with the + * outer one's restarts and must not destroy them either - the outer loop is + * still going to need them when the inner one resumes. Same reason [depth] is + * a count and not a flag. */ +static snapshot snaps[BREAK_MAX]; +static _Atomic int snap_depth; /* published last; 0 = none */ + +static snapshot *snap_top(void) { + int d = atomic_load(&snap_depth); + return d <= 0 ? NULL : &snaps[d - 1]; +} + +/* Called on the game thread with the stack held still. 0 if there is no room + * to nest, which the caller reports rather than serving a stale one. */ +static int32_t snap_gen; /* monotone; 0 is "no snapshot" */ + +static int snap_push(void) { + int d = atomic_load(&snap_depth); + if (d >= BREAK_MAX) return 0; + snapshot *s = &snaps[d]; + int32_t n = flan_restart_count(); + s->gen = ++snap_gen; + s->total = n; + s->used = 0; + s->n = 0; + for (int32_t i = 0; i < n && s->n < SNAP_MAX; i++) { + int64_t len = 0; + const uint8_t *nm = flan_restart_name(i, &len); + void *fr = flan_restart_frame(i); + if (nm == NULL || fr == NULL) continue; + if (len < 0) len = 0; + if ((int64_t)s->used + len + 1 > SNAP_NAMES) break; + s->frame[s->n] = fr; + s->off[s->n] = s->used; + s->len[s->n] = (int32_t)len; + /* The outermost [restart_floor] frames are below the thunk boundary. */ + s->reachable[s->n] = (i < n - restart_floor); + memcpy(s->names + s->used, nm, (size_t)len); + s->used += (int32_t)len; + s->names[s->used++] = 0; + s->n++; + } + atomic_store(&snap_depth, d + 1); + return 1; +} + +static void snap_pop(void) { + int d = atomic_load(&snap_depth); + if (d > 0) atomic_store(&snap_depth, d - 1); +} /* The condition's class name, so an editor can say what stopped rather than * only that something did. It is all there is to say: the hook is handed the * name and an opaque pointer, and nothing at run time can render a value whose @@ -135,16 +247,32 @@ static void break_loop(const uint8_t *name, int64_t namelen, void *condition, fflush(stdout); fprintf(stderr, "\nflan: unhandled %.*s — stopped, not dead.\n", (int)namelen, (const char *)name); + /* Taken before anything is printed, and before [depth] says there is a + * break to ask about: the list on the terminal and the list on the socket + * are then the same list, numbered the same way, and the numbers are what a + * choice is made of. */ + int32_t my_gen; + if (!snap_push()) { + fflush(stdout); + fprintf(stderr, "flan: %d nested break loops - giving up rather than " + "spinning\n", BREAK_MAX); + fflush(stderr); + exit(134); + } { - int32_t n = flan_restart_count(); - if (n == 0) + snapshot *s = snap_top(); + my_gen = s->gen; + if (s->n == 0) fprintf(stderr, " no restarts are active; abort, or fix and reload\n"); - for (int32_t i = 0; i < n; i++) { - int64_t len = 0; - const uint8_t *nm = flan_restart_name(i, &len); - if (nm != NULL) - fprintf(stderr, " restart: %.*s\n", (int)len, (const char *)nm); - } + for (int32_t i = 0; i < s->n; i++) + /* Numbered, because that is how one is taken now, and marked when it is + * not takeable - a restart below the thunk boundary is shown rather + * than hidden, since "why can I not have that one" is a fair question + * and silence is how this went wrong the first time. */ + fprintf(stderr, " %2d. restart: %s%s\n", i, s->names + s->off[i], + s->reachable[i] ? "" : " (below this break; cannot be taken)"); + if (s->total > s->n) + fprintf(stderr, " ... and %d more, not listed\n", s->total - s->n); } fflush(stderr); /* What the *outer* loop was reporting, restored on the way out: resuming an @@ -183,13 +311,18 @@ static void break_loop(const uint8_t *name, int64_t namelen, void *condition, * arriving during the attempt passes its own check, writes a new name * and sets the flag, and the store below then erases it. Copying also * keeps strlen off a buffer the listener may be writing. */ - char take[sizeof chosen]; - memcpy(take, chosen, sizeof take); + /* Read before the claim: a choice addressed to some other break is + * not this one's to consume. */ + if (atomic_load(&chosen_gen) != my_gen) { nanosleep(&step, NULL); continue; } + int take = atomic_load(&chosen_index); atomic_store(&chosen_ready, 0); - int32_t ok = - flan_break_resume((const uint8_t *)take, (int64_t)strlen(take), xfer); + snapshot *s = snap_top(); + int ok = s != NULL && s->gen == my_gen && take >= 0 && take < s->n + && s->reachable[take]; if (ok) { - fprintf(stderr, "flan: resuming at restart %s\n", take); + flan_restart_take(s->frame[take], xfer); + fprintf(stderr, "flan: resuming at restart %d. %s\n", take, + s->names + s->off[take]); fflush(stderr); memcpy(condition_name, outer_name, sizeof condition_name); /* Cleared with the resume: an abort that passed its check just as the @@ -197,10 +330,18 @@ static void break_loop(const uint8_t *name, int64_t namelen, void *condition, * at the *next* unhandled error, minutes later, in unrelated code, * giving nobody the chance to choose. */ atomic_store(&aborting, 0); + /* Popped *before* the depth comes down. The other order leaves a + * window where [depth] says the outer break is the current one and + * [snap_top] still answers with the inner one's list, so a request + * arriving in it is validated against a list nobody is looking at. */ + snap_pop(); atomic_fetch_sub(&depth, 1); return; } - fprintf(stderr, "flan: no restart named %s is active\n", take); + /* The listener checks all of this before answering ok, so reaching here + * means the two disagreed - worth saying loudly rather than looping on + * in silence, which is the failure this whole change is about. */ + fprintf(stderr, "flan: restart %d is not one this break can take\n", take); fflush(stderr); } nanosleep(&step, NULL); @@ -224,8 +365,19 @@ int32_t flan_agent_poll(void) { job j = queue[t % QUEUE]; atomic_store_explicit(&tail, t + 1, memory_order_relaxed); if (j.install != NULL) { j.install(); n++; } - /* After the install, so a thunk sees the bodies its own module published. */ - if (j.call != NULL) { j.call(); } + /* After the install, so a thunk sees the bodies its own module published. + * + * The floor moves for the duration. [flan_reload_call] holds its own + * transfer channel and drops it on return, so every restart frame that + * was on the stack before this call is unreachable from a break inside + * it. Saved and restored rather than set and cleared: this runs from + * inside break loops, which run from inside thunks. */ + if (j.call != NULL) { + int32_t outer = restart_floor; + restart_floor = flan_restart_count(); + j.call(); + restart_floor = outer; + } if (j.handle != NULL) { dlclose(j.handle); } } } @@ -307,43 +459,103 @@ static void serve(int fd) { reply(fd, "running\n"); return; } + /* One line per restart, innermost first: the index it is taken by, a flag + * for whether it can be taken at all, and the name. The index leads + * because it is the identity - two frames can offer [retry] and only one + * of them is the one meant, which is the whole reason this is not a list + * of names any more. Read from the snapshot, never from the live stack. */ if (strcmp(line, "restarts") == 0) { if (!(atomic_load(&depth) > 0)) { reply(fd, "err not stopped\n"); return; } - int32_t n = flan_restart_count(); - for (int32_t i = 0; i < n; i++) { - int64_t len = 0; - const uint8_t *nm = flan_restart_name(i, &len); - if (nm != NULL) { - send(fd, nm, (size_t)len, MSG_NOSIGNAL); - reply(fd, "\n"); - } + snapshot *s = snap_top(); + if (s == NULL) { reply(fd, "err no restart snapshot\n"); return; } + for (int32_t i = 0; i < s->n; i++) { + char hdr[32]; + int k = snprintf(hdr, sizeof hdr, "%d %c ", i, + s->reachable[i] ? '+' : '-'); + if (k > 0) send(fd, hdr, (size_t)k, MSG_NOSIGNAL); + send(fd, s->names + s->off[i], (size_t)s->len[i], MSG_NOSIGNAL); + reply(fd, "\n"); } reply(fd, ".\n"); return; } - if (strncmp(line, "restart ", 8) == 0) { + /* Take the i'th, optionally checking that the caller and this snapshot + * still agree on what the i'th is called. The name is not the lookup - + * that is the bug - it is a receipt: a client that listed, prompted, and + * chose has the name in hand for nothing, and sending it turns a bare + * integer into something that can be wrong out loud. */ + if (strncmp(line, "restart-at ", 11) == 0) { if (!(atomic_load(&depth) > 0)) { reply(fd, "err not stopped\n"); return; } - size_t k = strlen(line + 8); - if (k == 0 || k >= sizeof chosen) { reply(fd, "err bad restart name\n"); return; } - /* Checked here, against the stack the stopped thread is holding still, - * rather than accepted and found wrong after the reply has gone. "ok" - * has to mean the program will resume. */ - { - int32_t n = flan_restart_count(), found = 0; - for (int32_t i = 0; i < n && !found; i++) { - int64_t len = 0; - const uint8_t *nm = flan_restart_name(i, &len); - found = nm != NULL && (size_t)len == k && memcmp(nm, line + 8, k) == 0; - } - if (!found) { - reply(fd, "err no restart named "); - reply(fd, line + 8); - reply(fd, " is active\n"); + snapshot *s = snap_top(); + if (s == NULL) { reply(fd, "err no restart snapshot\n"); return; } + char *end = NULL; + long idx = strtol(line + 11, &end, 10); + if (end == line + 11) { reply(fd, "err restart-at wants an index\n"); return; } + if (idx < 0 || idx >= s->n) { + reply(fd, "err no restart at that index\n"); + return; + } + while (*end == ' ') end++; + if (*end != '\0') { + size_t k = strlen(end); + if (k != (size_t)s->len[idx] || + memcmp(end, s->names + s->off[idx], k) != 0) { + reply(fd, "err that index is now "); + reply(fd, s->names + s->off[idx]); + reply(fd, ", not what you named; list the restarts again\n"); return; } } - memcpy(chosen, line + 8, k + 1); - /* Published last, so the game thread never reads a half-written name. */ + if (!s->reachable[idx]) { + /* Refused, with the reason, rather than accepted and dropped. The + * transfer would unwind to the thunk this break is inside and stop + * there, and the program would carry on as if nothing had been + * chosen. */ + reply(fd, "err restart "); + reply(fd, s->names + s->off[idx]); + reply(fd, " is below the evaluation this break is inside, so a " + "transfer to it has nowhere to land; choose one offered " + "above it, or abort\n"); + return; + } + atomic_store(&chosen_index, (int)idx); + atomic_store(&chosen_gen, s->gen); + /* Published last, so the game thread never reads an index that is about + * to change, or one whose generation has not arrived yet. */ + atomic_store(&chosen_ready, 1); + reply(fd, "ok\n"); + return; + } + /* By name, still, for a person at a raw socket - and now defined as + * exactly [restart-at] on the first index offering the name, which is + * what §4's walk already meant. So the two verbs cannot disagree, and a + * shadowed name is reachable through the other one rather than nowhere. */ + if (strncmp(line, "restart ", 8) == 0) { + if (!(atomic_load(&depth) > 0)) { reply(fd, "err not stopped\n"); return; } + snapshot *s = snap_top(); + if (s == NULL) { reply(fd, "err no restart snapshot\n"); return; } + size_t k = strlen(line + 8); + if (k == 0) { reply(fd, "err bad restart name\n"); return; } + int32_t at = -1; + for (int32_t i = 0; i < s->n && at < 0; i++) + if ((size_t)s->len[i] == k && memcmp(s->names + s->off[i], line + 8, k) == 0) + at = i; + if (at < 0) { + reply(fd, "err no restart named "); + reply(fd, line + 8); + reply(fd, " is active\n"); + return; + } + if (!s->reachable[at]) { + reply(fd, "err restart "); + reply(fd, line + 8); + reply(fd, " is below the evaluation this break is inside, so a " + "transfer to it has nowhere to land; choose one offered " + "above it, or abort\n"); + return; + } + atomic_store(&chosen_index, at); + atomic_store(&chosen_gen, s->gen); atomic_store(&chosen_ready, 1); reply(fd, "ok\n"); return;