JosephFerano/flan
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge branch 'worktree-agent-a19373b804f289af0' into dev-loop

Browse Source
This commit is contained in:
Joseph Ferano 2026-09-13 12:19:20 +07:00
commit bbd1c67474
5 changed files with 299 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

142
HANDOFF-f2.md Normal file
View File

@ -0,0 +1,142 @@
# Handoff: `pause` marking from Emacs (DISCUSS.md §9)
The daemon half is built and works end to end over the wire. The Emacs half is **not** built.
Nothing half-written was left behind: `dune build` is clean and there is no new test.
## The design, as it stands after reading the code
`C-u` before an eval marks a form so the program stops when that form runs. The mark is **not**
spliced into the source text — that would move every line and column after the insertion, and the
error overlays, `layout`, the break loop's frame locations and DWARF all read those. Instead the
editor sends a **position** beside the code:
```
(:op "eval" :code "(defn step [] i64 ...)" :file "/x/y.flan" :pause (LINE COL))
```
The daemon parses and `Load`s as usual, then walks the resulting `Ast.decl list` and puts a
`(pause)` call at whatever *starts* at that position. `(pause)` is an ordinary prelude function
(`error` under a `restart-case` with a `continue` clause), so an instrumented body is just a body
that calls one more function, and the break loop it lands in is the one an unhandled condition
already builds. Nothing in the compiler changes.
It **sticks** with no extra machinery: the marked declaration is what goes into `Session.t.decls`,
so it stays marked until an evaluation replaces it — an ordinary `C-c C-c` over the same form with
no `:pause`, or `C-c C-k` over the buffer. That is §9's settled behaviour and it costs one
statement that was already there.
### What §9 left out or got slightly wrong
- **§9 says "send the top-level form with that span replaced by `(do (pause) <span>)`".** That
wrapping is right for a sub-expression but *impossible* for the first of its three targets: a
whole top-level `defn` is a declaration, and `(do (pause) (defn ...))` is not an expression.
Marking a whole `defn` therefore means *stopping on entry*, and the call goes at the front of
`fbody`. `Ast.mark_pause` does both, chosen by what the position lands on.
- **§9 does not say the mark can be refused.** It has to be: a position that matches nothing must
be an error, because installing an unmarked body and answering `ok` reports a breakpoint that is
not there — the silent-success failure the session refuses everywhere else.
- **Desugaring makes locations non-unique.** `parse.ml` gives several nested nodes the same
location (`when` becomes an `If` whose branch is a `Do` at the `when`'s own position — lines 141,
164, 651, 654). The walk is pre-order and stops at the first hit, so the outermost node at that
position wins, which is the one the editor pointed at.
- **The third target ("the form point is inside") needs no new daemon work** — it is the same
position field, computed differently in Emacs.
## What was built, file by file
All four are **working** (built, and exercised against a real daemon and a real running program by
hand — see "How it was checked").
- **`lib/ast.ml`** — new section at the end:
- `map_children : (expr -> expr) -> expr -> expr`, an exhaustive one-level rebuild. Exhaustive on
purpose: a missing constructor is a form you silently cannot stop inside.
- `pause_call : Loc.t -> expr` — `(pause)` at a given location.
- `mark_pause : line:int -> col:int -> decl list -> decl list option` — pre-order, first hit wins,
`None` when nothing is at that position. The synthesized `Do`/`Call` take the target's own
location, never `Loc.unknown`, because DWARF and the break loop's frame location read it.
- **`lib/wire.ml`** — `pos_field form key`, reading `(LINE COL)` as a pair of ints; `None` for
anything else, the same narrowness as `int_field`.
- **`lib/session.ml`** — `eval` takes `?pause:(int * int)`. It is applied **after**
`Load.qualify_decl`, so a package that defines a `pause` of its own cannot capture the
synthesized call, and a position that matches nothing is a `Loc.fail` naming the position.
- **`lib/dev.ml`** — `eval` takes `~pause`, `handle` reads `:pause` off the request, and a
successful install echoes `:pause "LINE:COL"` back so an editor marks the buffer only for a mark
the session actually applied. With no `:pause` in the request, every byte of the old behaviour is
unchanged.
## How it was checked
A daemon over `test/programs/dev-loop.flan`, driven by a raw socket client:
- eval of `step` with `:pause (1 1)` → `(:status "ok" … :pause "1:1")`, and a later `describe`
came back `:stopped t :condition "Pause"` — the program stopped, on the prelude's own condition.
- `:pause (1 999)` → `(:status "error" :message "nothing to pause at line 1, column 999 of the form
sent")`.
- a plain re-eval of the same form → accepted.
`dune build` is clean. **`dune test` was not run** (budget). The changes are additive: the new
session argument is optional and the new reply field only appears when `:pause` was sent, so no
existing path changes shape — but the suite should be run first thing next session anyway. Note
`test_dev.ml`'s first block is separately known-flaky (a socket bind race, ~1 in 4).
## What remains, in order
1. **`lib/dev.ml`, `eval_expr`** — accept `:pause` for `C-u C-x C-e` (§9's "last expression").
Two parts. (a) `lib/session.ml`'s `eval_expr` should take `?(pause = false)` and wrap
`Parse.expr form` in `Do [Ast.pause_call loc; e]` before `Check.expression`. (b) `eval_expr`'s
`wait` loop in `dev.ml` returns `error "the program did not reach a frame boundary…"` after 5s,
which is exactly what a thunk that stopped in the break loop will do — so it would report the
working feature as a failure. Make `wait` three-way (`` `Value | `Stopped | `Timeout ``) and
check `state t = Stopped` **only when a pause was requested**: `test_dev.ml:519–560` already
asserts the current timeout shape for the no-pause case (`"an expression that stopped inside a
break answered anyway"`), and that must stay byte-identical.
Use `:pause t` here, not `(LINE COL)`: `flan-eval-last-sexp` sends a raw `buffer-substring`
with no line padding (unlike `flan-dev--text`), so buffer coordinates do not survive that path.
2. **`emacs/flan-dev.el`, `flan-dev--eval`** — take an optional pause position and put
`:pause (LINE COL)` on the request. The column is a **1-based byte offset**, per the comment
above `flan-dev--position`: `(1+ (- (position-bytes pos) (position-bytes (line-beginning-position))))`,
*not* `current-column`. The line is the buffer's own line, which already works because
`flan-dev--text` pads with leading newlines.
3. **`emacs/flan-dev.el`, `flan-eval-defun`** — `(interactive "P")`. `C-u` marks the innermost form
point is inside (`backward-up-list`, falling back to the defun's start when point is not nested);
`C-u C-u` marks the top-level form itself, i.e. stop on entry. That plus item 1 covers §9's three
targets with no new keybinding — `emacs/flan-mode.el` needs no change.
4. **`emacs/flan-dev.el`, the visual indication** — a `flan-dev-pause-face` overlay over the marked
form's bounds, drawn **only** when the reply carries `:pause`, tagged with a `flan-dev-pause`
property. Copy the shape of the error overlays (`flan-dev--show-error`) but *not* their lifetime:
a pause mark is an annotation on the program, not feedback about one command, so it must survive
`pre-command-hook`. Remove overlays intersecting the sent region on every accepted plain eval,
and over the whole buffer in `flan-eval-buffer` — that is the visible half of "cleared by an
ordinary `C-c C-c`".
5. **`test/programs/dev-pause.flan`** — new, shaped like `dev-break.flan`'s tail so the marked
function keeps being called: `(dotimes [i 4000] (agent/wait 5) (set ticks (step)))`.
`dev-loop.flan` calls `step` only four times, which is too tight.
6. **`test/test_dev.ml`** — a block in the style of the break-loop block (its own daemon, its own
program, its own output buffer). Assert both halves: mark `step`'s `(+ ticks 1)` sub-expression
→ await `:stopped t` with a condition containing `Pause` → `break` lists `continue` → then
**re-eval `step` plainly, take `continue`, and confirm it runs on without stopping again**. The
second assertion is the one that tests the settled "it sticks until evaluated plainly" decision
and the one most likely to be skipped.
7. **`emacs/test-flan-dev.el`** — the elisp side, once items 2–4 exist.
## Decisions made that were not already settled
- **The mark is not a field on `Ast`.** It is an ordinary `(pause)` call spliced into the tree. A
`paused : bool` on `Ast.expr` would have to be threaded through `Check`, `Tast` and `Emit` for a
feature the prelude already implements as a function.
- **No parallel `paused` list on `Session.t`.** The spliced declaration in `t.decls` *is* the state
that makes the mark stick; a second `(name * position) list` would be a second source of truth
that drifts the first time some path replaces `decls` without touching it. Clearing then falls out
for free — any plain eval replaces the stored declaration with an unmarked one.
- **Marking a whole `defn` means stopping on entry**, since it cannot be wrapped (above).
- **A position matching nothing is refused**, rather than installed unmarked (above).
- **The splice happens after qualification**, so a package's own `pause` cannot capture it.
- **The reply echoes `:pause "LINE:COL"`** so the editor draws its overlay off the daemon's
confirmation and can never claim a mark that was refused.
## Tried and abandoned
- Nothing failed outright. Worth recording: the worktree this was done in was created **485 commits
behind** `dev-loop` (at `2c232dd`, before `lib/dev.ml` existed at all) and had to be
`git reset --hard` to the branch tip before any of the files named in the task existed. Check
`git log` against `dev-loop` before starting in a fresh worktree.

104
lib/ast.ml
View File

@ -175,3 +175,107 @@ let declared_name (d : decl) =
| Defvar (n, _, _) | Defconst (n, _, _) -> Some n
| Declare (fn, _) | DeclareC (fn, _) | Defn fn -> Some fn.name
| Package _ | Import _ -> None
(* ── Instrumenting a form with (pause) ─────────────────────────────── *)
(* [C-u C-c C-c] marks a form so the program stops when it runs — DISCUSS.md
§9. The mark travels beside the source as a position and is applied *here*,
to the AST, rather than being spliced into the text the editor sends: text
would shift every line and column after the insertion, and the error
overlays, the layout, the break loop's frame locations and DWARF all read
those. Applied after parsing, every location is already attached and none of
them moves.
Nothing in the compiler knows about this. [(pause)] is an ordinary prelude
function — [error] under a [restart-case] — so an instrumented body is a
body that calls one more function, and the break loop it lands in is the one
an unhandled condition already builds. *)
(* Rebuild [e] with [f] applied to each expression written directly inside it.
Exhaustive on purpose: a constructor left out would be a form the mark
silently cannot be set inside, which is the kind of hole nobody finds
except by trying it on the one function they wanted to stop in. *)
let map_children f (e : expr) : expr =
let ex = f in
let bind (b : binding) = { b with bval = ex b.bval } in
let arm (a : arm) = { a with body = List.map ex a.body } in
let hcl (h : hclause) = { h with hbody = List.map ex h.hbody } in
let rcl (r : rclause) = { r with rbody = List.map ex r.rbody } in
let place = function
| Pvar n -> Pvar n
| Pfield (x, n) -> Pfield (ex x, n)
| Pindex (x, is) -> Pindex (ex x, List.map ex is)
| Pderef x -> Pderef (ex x)
in
let kind =
match e.e with
| Int _ | Float _ | Byte _ | Str _ | Kw _ | Quote _ | Var _ | ArrayOf _
| Break _ | Continue _ -> e.e
| Do es -> Do (List.map ex es)
| Let (bs, es) -> Let (List.map bind bs, List.map ex es)
| If (c, a, b) -> If (ex c, ex a, Option.map ex b)
| While (l, c, es) -> While (l, ex c, List.map ex es)
| Loop (bs, es) -> Loop (List.map (fun (n, v) -> (n, ex v)) bs, List.map ex es)
| Recur es -> Recur (List.map ex es)
| Return x -> Return (Option.map ex x)
| Set (p, v) -> Set (place p, ex v)
| Field (x, n) -> Field (ex x, n)
| Call (fn, args) -> Call (ex fn, List.map ex args)
| Match (s, arms) -> Match (ex s, List.map arm arms)
| Struct (n, fs) -> Struct (n, List.map (fun (n, v) -> (n, ex v)) fs)
| Arr es -> Arr (List.map ex es)
| Fn (ps, es) -> Fn (ps, List.map ex es)
| Dotimes (l, n, c, es) -> Dotimes (l, n, ex c, List.map ex es)
| Defer es -> Defer (List.map ex es)
| Unwrap (u, x) -> Unwrap (u, ex x)
| HandlerBind (cs, es) -> HandlerBind (List.map hcl cs, List.map ex es)
| Signal (k, x) -> Signal (k, ex x)
| RestartCase (b, cs) -> RestartCase (ex b, List.map rcl cs)
| InvokeRestart (n, args) -> InvokeRestart (n, List.map ex args)
in
{ e with e = kind }
let pause_call loc = { e = Call ({ e = Var "pause"; loc }, []); loc }
(* [mark_pause ~line ~col ds] is [ds] with a [(pause)] put in front of whatever
starts at that position, or [None] when nothing does.
[None] rather than "leave it alone": installing an unmarked body and
answering "ok" would report a breakpoint that is not there, which is the
failure the session refuses everywhere else.
Pre-order, and it stops at the first hit. Desugaring gives several nested
nodes the same location — [(when c a)] becomes an [If] whose else-less
branch is a [Do] at the [when]'s own position — so the outermost of those is
the one the editor pointed at.
A whole top-level [defn] is the third target from §9 and cannot be wrapped:
[(do (pause) (defn ...))] is not an expression. Marking one means stopping
on entry, so the call goes at the front of its body. *)
let mark_pause ~line ~col (ds : decl list) : decl list option =
let at (l : Loc.t) = l.Loc.line = line && l.Loc.col = col in
let hit = ref false in
let rec walk (e : expr) =
if !hit then e
else if at e.loc then begin
hit := true;
(* The [Do] takes the target's own location, and the target keeps its
own: a wrapper at [Loc.unknown] would put the frame the break loop
reports, and the line DWARF names, nowhere. *)
{ e with e = Do [ pause_call e.loc; e ] }
end
else map_children walk e
in
let body es = List.map walk es in
let decl (d : decl) =
match d.d with
| Defn f when (not !hit) && at d.dloc ->
hit := true;
{ d with d = Defn { f with fbody = pause_call d.dloc :: f.fbody } }
| Defn f -> { d with d = Defn { f with fbody = body f.fbody } }
| Defvar (n, t, Init e) -> { d with d = Defvar (n, t, Init (walk e)) }
| Defconst (n, t, e) -> { d with d = Defconst (n, t, walk e) }
| _ -> d
in
let ds = List.map decl ds in
if !hit then Some ds else None

21
lib/dev.ml
View File

@ -418,10 +418,13 @@ let error ?loc msg =
^ (match loc with None -> "" | Some l -> " :loc " ^ Wire.quote l)
^ ")"
let eval t ~code ~origin =
(* [pause], when given, is the position of the form to stop at — §9. It rides
beside the code rather than in it, and the reply echoes it back so an editor
marks the buffer only for a mark the session actually applied. *)
let eval t ~code ~origin ~pause =
if not (alive t) then error "the program exited; restart flan dev"
else
match Session.eval ~origin t.session code with
match Session.eval ~origin ?pause t.session code with
| c when not c.Session.installs ->
(* Accepted into the session and nothing to send: a declaration the
program already has, with no body and no new storage. Saying "ok" and
@ -454,10 +457,14 @@ let eval t ~code ~origin =
{ ogen = t.gen; oso = out; oll = ll; oloc = fn_loc t n })
c.Session.fns;
ok
[ ":names " ^ Wire.strings c.Session.names;
":fns " ^ Wire.strings c.Session.fns;
Printf.sprintf ":ms %.1f"
(timing.Build.llc_ms +. timing.Build.link_ms) ]
([ ":names " ^ Wire.strings c.Session.names;
":fns " ^ Wire.strings c.Session.fns;
Printf.sprintf ":ms %.1f"
(timing.Build.llc_ms +. timing.Build.link_ms) ]
@ (match pause with
| Some (l, c) ->
[ ":pause " ^ Wire.quote (Printf.sprintf "%d:%d" l c) ]
| None -> []))
| reply -> error ("the program refused the module: " ^ reply)
| exception Unix.Unix_error (e, _, _) ->
error
@ -1664,7 +1671,7 @@ let handle t req =
let origin =
match Wire.string_field req "file" with Some f -> f | None -> "<editor>"
in
eval t ~code ~origin
eval t ~code ~origin ~pause:(Wire.pos_field req "pause")
| None -> error "eval needs :code")
| Some "eval-expr" ->
(match Wire.string_field req "code" with

27
lib/session.ml
View File

@ -270,7 +270,18 @@ type change = {
installs : bool;
}
let eval ?(origin = "<eval>") t src : change =
(* [pause] is [C-u C-c C-c]: the position, in the source just sent, of the form
the program should stop at — DISCUSS.md §9. It arrives as a separate field
rather than spliced into [src], because splicing text would move every
location after it, and it is applied below to the *declarations*, once
parsing has attached those locations and [Load] has qualified the names.
Nothing here makes it stick and nothing has to: the marked declaration is
what goes into [t.decls], so it stays marked until an evaluation replaces
it — which is an ordinary [C-c C-c] over the same form, with no [:pause].
That is §9's settled behaviour, and it is the same one statement that
accepts every other change. *)
let eval ?(origin = "<eval>") ?pause t src : change =
let forms = Reader.read_all ~file:origin src in
(* Through [Load] like any other source, so an evaluated (import ...) means
what it means in a file. Its expansion is what gets spliced, which is also
@ -296,6 +307,20 @@ let eval ?(origin = "<eval>") t src : change =
let loc =
match incoming with d :: _ -> d.Ast.dloc | [] -> Loc.unknown
in
(* After [qualify_decl], so a package that defines a [pause] of its own does
not capture the call this splices in. Refused when the position matches
nothing: installing an unmarked body and answering "ok" would report a
breakpoint that is not there. *)
let incoming =
match pause with
| None -> incoming
| Some (line, col) ->
(match Ast.mark_pause ~line ~col incoming with
| Some ds -> ds
| None ->
fail loc "nothing to pause at line %d, column %d of the form sent"
line col)
in
let names = List.filter_map Ast.declared_name incoming in
let replacement n =
List.find_opt

13
lib/wire.ml
View File

@ -103,3 +103,16 @@ let parse src =
match Reader.read_all ~file:"<wire>" src with
| [ f ] -> f
| _ -> Loc.fail Loc.unknown "one form per message"
(* [:pause (LINE COL)] — where in the source just sent a [(pause)] goes. A
position and not a span: the daemon matches it against the location the
reader already attached to that form, so the editor says *which* form by
saying where it starts. Anything that is not two integers is [None], and
the op says what it wanted, exactly as [int_field] does. *)
let pos_field form key =
match field form key with
| Some { Form.v =
Form.List [ { Form.v = Form.Int l; _ }; { Form.v = Form.Int c; _ } ];
_ } ->
Some (Int64.to_int l, Int64.to_int c)
| _ -> None