From 6bc4726dddeedfcab9bf2e839607fb763fc89e98 Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Sun, 13 Sep 2026 17:54:36 +0700 Subject: [PATCH] C-x C-e expands, and a declaration is not an expression Parse.expr never ran the expander, so a macro call typed as a bare expression was an unknown name -- a package's and the prelude's alike, which is what said the gap was older than importable macros. It is the wrap Parse.decl already had, applied to the other entry point, with Parse.with_imported in front of it in Session.eval_expr because the one expression an editor sends carries no import. The decision that was waiting: an expression that expands to a declaration is refused by name, in the head dispatch rather than in a walk over what the expander answered, so a nested one and a hand-typed one get the same sentence. A quasiquoted declaration is still a value. The spin refusal fires on this path; the ring cannot reach it, because a ring is refused while its own package is parsed. Expansion happens before the thunk is built, so the 5s three-way wait is untouched. --- BUILT.md | 43 ++++++++++++++-- NEXT.md | 14 ++--- lib/parse.ml | 48 ++++++++++++++++- lib/session.ml | 13 ++++- test/programs/pkg-macro-idle.flan | 17 ++++++ test/test_repl.ml | 16 +++++- test/test_session.ml | 86 +++++++++++++++++++++++++++++++ 7 files changed, 224 insertions(+), 13 deletions(-) create mode 100644 test/programs/pkg-macro-idle.flan diff --git a/BUILT.md b/BUILT.md index 2a14e59..5fecf56 100644 --- a/BUILT.md +++ b/BUILT.md @@ -671,9 +671,46 @@ macro in a package and reload the file that imports it: the session has been hol quietest failure in this area and `test_session` pins it in both places — the reload itself, and the `C-c C-c` after the reload, which reads the set the session kept rather than the one `Load` handed it. -Not covered: `C-x C-e`. `Parse.expr` does not run the expander at all, so a macro call typed as a bare expression is an -unknown name — a package's and the prelude's alike, which is what says this is an older gap and not this feature's. -`Session.eval_expr` is the only path that reaches it. +### `C-x C-e` expands too, and a declaration is not an expression + +`Parse.expr` never ran the expander, so a macro call typed as a bare expression was an unknown name — a package's and +**the prelude's alike**, which is what said the gap was older than importable macros and not theirs. `(unless c a b)` +at `C-x C-e` failed exactly as `(mac/twice 4)` did. It is the wrap `Parse.decl` already had, applied to the other entry +point: quasiquote-desugar, expand, then parse the one form that comes back. `Session.eval_expr` puts +`Parse.with_imported` in front of it as `Session.eval` does, because the one expression an editor sends carries no +import and the session is the only thing holding what the imports brought in. + +**What it expands is the prelude's macros and the imported packages', and not the file's own.** That limit is the +session's rather than this path's: `Macro.program` collects a file's macros by scanning the forms it is handed, and the +forms handed to an evaluation are the one thing that was sent. `C-c C-c` has always had the same limit for the same +reason, and a `defmacro` typed at the REPL becomes an ordinary `Ast.Defn` that nothing records as a macro. Left where +it was rather than half-fixed here, and pinned in `test_session` so that changing it is a decision. + +**An expression that expands to a declaration is refused, by name.** `defn`, `defvar`, `defconst`, `defstruct`, +`defunion`, `defenum`, `defalias`, `defmacro` and `import` are heads `Parse.expr` now rejects — the arm that used to +say it for `defmacro` alone, generalised. It sits in the head dispatch and not in a walk over what the expander +answered, so it catches a declaration nested anywhere in the expansion for free, catches one **typed** by hand with the +same sentence instead of "unknown name defvar", and cannot drift out of sync with `decl`'s list the way a second copy +would. A *quasiquoted* declaration is deliberately not caught: after desugaring, the name in `` `(defn ...) `` is a +string inside a `Form.Sym` argument rather than a head — the same property that makes a quasiquoted call output rather +than a dependency. Building a declaration as a value is what a macro is for; evaluating one is not a thing an +expression can do. + +**The non-termination refusals, and where each of them is.** This matters more here than in a build: `eval_expr` runs +inside the daemon, and a hang there wedges the editor with the program still on screen. The **spin** — a macro that +expands into a call to itself and does not get smaller — fires on this path, bounded at 200 rounds, and comes back as +a `Loc.Error` that `Dev.eval_expr` already answers as an error reply. The **ring** never reaches this path, and +finding out why was worth the trip: a ring is refused while *the package it lives in* is parsed, because that file's +own bodies name each other, so no importer of a ring can be loaded and no session over one can exist. The refusal is +in front of the path rather than on it, which is the stronger place for it; `test_session` asserts it at +`Session.create` so that moving the check later shows up as a failing test and not as a wedged daemon. +`test/programs/pkg-macro-idle.flan` is the fixture the spin needs — it imports and calls nothing, so the session is +created without expanding anything and the expression is the first thing that ever expands the macro. + +Expansion happens **before** the thunk is built, so the three-way `` `Value | `Stopped | `Timeout `` wait in +`Dev.eval_expr` is untouched: a cold macro module costs its ~300ms before that 5-second clock starts, and `before` is +sampled after `Session.eval_expr` has returned. `temps` is not reset on this path, unlike `decl`'s — a declaration is +a fresh top level, an expression is evaluated into a session that has been handing out temporaries all along. Still missing: a package-private marker for anything other than `main`, which is why `rl/get-color-raw` is callable. The gap is surface syntax and not `load.ml` — `exported` is one predicate and the refusal machinery that points at the diff --git a/NEXT.md b/NEXT.md index a131af4..183122e 100644 --- a/NEXT.md +++ b/NEXT.md @@ -108,12 +108,14 @@ import resolver at the Form level. It did not. The file being compiled is parsed too, so no shape of the feature could have left import resolution where it was — `Load.program` takes forms now and uses the one resolver that always existed. -One gap left, and it is older than this work: **`C-x C-e` expands no macros at all.** `Parse.expr` -never calls the expander, so `(unless ...)` typed as a bare expression is as much an unknown name as -`(mac/twice 4)` is — the prelude's macros fail there too, which is what says this is not the package -feature's doing. `Session.eval_expr` is the only path that reaches it and the fix is the wrap -`Parse.decl` already has. Left alone deliberately: it changes what an expression evaluation means, -which is a decision and not a repair. +~~One gap left, and it is older than this work: **`C-x C-e` expands no macros at all.**~~ **Done** — +see [`BUILT.md`](BUILT.md), "`C-x C-e` expands too, and a declaration is not an expression". It was +the wrap `Parse.decl` already had, and the decision it was waiting on came out as: an expression that +expands to a declaration is **refused by name**, in `Parse.expr`'s head dispatch, so a `defn` nested +anywhere in the expansion and a `defvar` typed by hand get the same sentence. The spin refusal fires +on this path; the ring never can, because a ring is refused while its own package is parsed. What +expands is the prelude's macros and the imported packages' — not the file's own, which is a session +limit `C-c C-c` shares and which is now pinned rather than fixed. Two things found while finishing it. `Session` held the imported macro set but **replaced** it on every evaluation, and the one form `C-c C-c` sends carries no import — so a package macro worked on diff --git a/lib/parse.ml b/lib/parse.ml index e498b2b..24c0ca8 100644 --- a/lib/parse.ml +++ b/lib/parse.ml @@ -443,8 +443,24 @@ and form f mk (head : Form.t) (args : Form.t list) : Ast.expr = fail f "~@x means nothing outside a quasiquote, and splices only into a \ list or a vector" - | Sym "defmacro" -> - fail f "defmacro is a top-level declaration, not an expression" + (* A declaration is not an expression, and this arm says so for all of them + rather than for [defmacro] alone. It used to be that one, because it was + the only head anyone typed by mistake; now that [Parse.expr] expands, a + macro can *produce* one, and the head this dispatches on is the only place + that sees it — the walk recurses, so a [defn] nested inside what a macro + answered is caught with the same message as one typed at the top. + + A quasiquoted declaration is deliberately not caught: after desugaring, + the name in ``(defn ...)`` is a string inside a [Form.Sym] argument and not + a head, which is the same property that makes a quasiquoted macro call + output rather than a dependency. Building a declaration as a value is what + a macro is for. *) + | Sym ("defmacro" | "defn" | "defvar" | "defconst" | "defstruct" | "defunion" + | "defenum" | "defalias" | "import" as name) -> + fail f + "%s is a top-level declaration, not an expression. A macro may answer \ + with one only where a declaration is expected, so this cannot come back \ + from evaluating an expression" name (* Recognised, deliberately unimplemented. Rejected rather than left to fall through to Call, where they would parse and mean nothing. *) @@ -1060,3 +1076,31 @@ let decl (f : Form.t) : Ast.decl = a macro name is not one of the heads it knows. *) Loc.fail f.loc "expanding this declaration produced %d of them" (List.length fs) + +(* Single-expression entry point: C-x C-e, and the tests that parse one + expression. It expands, which [Parse.expr] above does not and never did — + so a bare [(unless c a b)] typed at the REPL was an unknown name, the + prelude's macros included. That is the whole of the change here: the wrap is + the one [decl] has, applied to the other entry point. + + [Expand.quasiquote] first for [parse_forms]'s reason — it is pure, it needs + nothing loaded, and the arm in [form] that refuses an undesugared quasiquote + is the backstop for the path that skips this, not for this one. + + What an expression that expands to a declaration does is decided in [form]: + [defn] and its siblings are refused by name, wherever in the expansion they + appear. Nothing here has to look for them. + + [temps] is deliberately not reset. [decl] resets it because a declaration is + a fresh top level; an expression is evaluated into a session that has been + handing out temporaries all along, and restarting the counter would hand out + a name the frame beside it is already using. *) +let expr (f : Form.t) : Ast.expr = + match !expander [ Expand.quasiquote f ] with + | [ f ] -> expr f + | fs -> + (* One expression in, one out. [Macro.program] is a [List.map], so it + cannot answer with anything else — this is here because the invariant is + worth stating where it is relied on, not because it has been seen. *) + Loc.fail f.loc "expanding this expression produced %d forms, and an \ + expression is one" (List.length fs) diff --git a/lib/session.ml b/lib/session.ml index 97dc426..92a1ce9 100644 --- a/lib/session.ml +++ b/lib/session.ml @@ -1091,7 +1091,18 @@ let eval_expr ?(origin = "") ?(pause = false) t src : change = | [] -> fail Loc.unknown "nothing to evaluate" | _ :: f :: _ -> fail f.Form.loc "one expression at a time" in - let parsed = Parse.expr form in + (* [Parse.expr] expands, so the imported set has to be in front of it here + exactly as [eval] puts it in front of a declaration: C-x C-e sends one + expression with no import in sight, and the session is the only thing + holding what the imports brought in. Without this the prelude's macros + would work and a package's would be an unknown name. + + Expansion happens here, before the thunk is built and long before the + agent is asked for anything, so the three-way wait in [Dev.eval_expr] is + untouched: a cold macro module costs its ~300ms before that clock starts, + and the non-termination refusals raise [Loc.Error] out of this call, which + the daemon already answers as an error rather than a silence. *) + let parsed = Parse.with_imported t.macros (fun () -> Parse.expr form) in (* Wrapped before the checker, so the call is checked like any other and a prelude that stopped offering [pause] would be an ordinary unknown name rather than a thunk that silently did not stop. The [Do] takes the diff --git a/test/programs/pkg-macro-idle.flan b/test/programs/pkg-macro-idle.flan new file mode 100644 index 0000000..e8e4c00 --- /dev/null +++ b/test/programs/pkg-macro-idle.flan @@ -0,0 +1,17 @@ +;;;; A file that imports a macro package and calls nothing from it, so it +;;;; builds. It exists for C-x C-e. +;;;; +;;;; A refusal that fires while the program is being built proves nothing about +;;;; the expression path: the session would never be created. Here the session +;;;; creates cleanly -- [Macro.program] scans and finds no call, so no module is +;;;; compiled -- and the first thing that ever expands [s/spin] is an expression +;;;; typed at the REPL. +;;;; +;;;; The ring is deliberately not imported here: a ring is refused while the +;;;; *package* is parsed, so no importer of one can be loaded at all and no +;;;; session over it can exist. See the assertion beside this one. + +(import s "pkgs/macspin") +(import mac "pkgs/mac") + +(defn main [] i32 0) diff --git a/test/test_repl.ml b/test/test_repl.ml index bf607e7..1a57366 100644 --- a/test/test_repl.ml +++ b/test/test_repl.ml @@ -152,6 +152,14 @@ let () = has to be *evaluated* and then reported as (). Emitting the literal without running it made the prompt answer while nothing happened. *) value "a call made for its effect" "(println \"printed\")" "()"; + (* A macro, over the socket. [Parse.expr] ran no expander at all until + now, so this was an unknown name — the prelude's macros included, + which is what said the gap was older than importable macros. [clamp] + is a prelude [defmacro] and [unless] is the one that stopped being a + special form, so between them they cover both shapes: one that + answers a value and one that answers unit. *) + value "a prelude macro" "(clamp 9 0 3)" "3"; + value "a prelude macro for its effect" "(unless false 1 2)" "()"; (* The one that proves it ran inside the process: the program increments [ticks] every frame, so two evaluations of it must disagree. A copy @@ -178,7 +186,13 @@ let () = | Some m -> fail "%s said %S, wanted it to mention %S" name m reason | None -> fail "%s was accepted" name in - refuses "a declaration" "(defvar nope i64)" ""; + (* A declaration is refused by name. It used to come back as "unknown + name defvar", which is why the reason asserted here was empty; now + that an expression expands, a macro can produce one, and the head + says what it is wherever it appears. *) + refuses "a declaration" "(defvar nope i64)" "top-level declaration"; + refuses "a declaration inside an expression" "(do 1 (defn f [] i32 1))" + "top-level declaration"; refuses "an unknown name" "no-such-name" "unknown name"; (* And the session is untouched by all of it: an evaluation is not a diff --git a/test/test_session.ml b/test/test_session.ml index 92d82b1..458b851 100644 --- a/test/test_session.ml +++ b/test/test_session.ml @@ -302,6 +302,92 @@ let () = (fun d -> try Unix.rmdir d with Unix.Unix_error _ -> ()) [ pkg; tmp ]; + (* ── C-x C-e expands, which it never used to ──────────────────────── + [Parse.expr] did not call the expander at all, so an expression typed at + the REPL saw no macros — not a package's and not the prelude's, which is + what said the gap was older than importable macros and not theirs. Both + halves are asserted here, in the session that [Dev.eval_expr] drives. + + Asserting on the IR and not merely on the absence of an exception: an + expression that did not expand is an unknown name and therefore raises, + but an expression that expanded to the wrong thing does not, and the + arithmetic the macro chose is the only witness of which happened. *) + (match Session.eval_expr ~origin:"programs/pkg-macro.flan" tm "(mac/twice 21)" with + | c -> + if not (has c.Session.ir "21, 21") then + fail "a package macro through C-x C-e did not expand to its body" + | exception Loc.Error { Loc.dmsg = m; _ } -> + fail "a package macro through C-x C-e: %s" m); + (* The prelude's, which is the case that says this was always broken. *) + (match Session.eval_expr ~origin:"programs/pkg-macro.flan" tm "(unless false 1 2)" with + | _ -> () + | exception Loc.Error { Loc.dmsg = m; _ } -> + fail "a prelude macro through C-x C-e: %s" m); + (* Not the file's own macro, and deliberately not: [Macro.program] collects + those by scanning the forms it is handed, and the forms handed to an + evaluation are the one thing that was sent. That limit is the session's + and not this path's — C-c C-c has always had it too, for the same reason — + so it is left where it is rather than half-fixed here. Pinned so that the + day it changes, it changes on purpose. *) + (match Session.eval_expr ~origin:"programs/pkg-macro.flan" tm "(tenfold 7)" with + | _ -> fail "the file's own macro expanded in a session — a welcome change, \ + but BUILT.md says it does not" + | exception Loc.Error _ -> ()); + + (* An expression that expands to a declaration. A macro may build one as a + value — that is what a quasiquote is for — but nothing can evaluate one, + so it is refused by name rather than arriving at the checker as an unknown + function called [defn]. Hand-typed here; the expanded case is the same + arm, because [Parse.expr] recurses and the head is the head either way. *) + (match Session.eval_expr ~origin:"programs/pkg-macro.flan" tm "(defn f [] i32 1)" with + | _ -> fail "a declaration was accepted as an expression" + | exception Loc.Error { Loc.dmsg = m; _ } -> + if not (has m "top-level declaration") then + fail "a declaration as an expression said %S" m); + (match Session.eval_expr ~origin:"programs/pkg-macro.flan" tm "(do 1 (defvar g i64))" with + | _ -> fail "a nested declaration was accepted as an expression" + | exception Loc.Error { Loc.dmsg = m; _ } -> + if not (has m "top-level declaration") then + fail "a nested declaration as an expression said %S" m); + + (* The two non-termination refusals. They matter more here than in a build: + [eval_expr] runs inside the daemon, and a hang there wedges the editor + with the program still on screen and no way to say so. What has to be true + is that neither can loop before the wait in [Dev.eval_expr] begins — both + come back as [Loc.Error], which the daemon already answers as an error. + + The spin is the one that fires on this path. [pkg-macro-idle.flan] imports + its package and calls nothing, so the session is created without expanding + anything — a fixture that called it would fail at [Session.create] and + prove nothing about an expression. The macro compiles, runs, and is + stopped by the fuel, here and not earlier. + + The ring is the one that cannot be reached from here, and finding out why + is the useful part: a ring is refused while the *package it lives in* is + parsed, because that file's own bodies name each other. So no importer of + a ring can be loaded and no session over one can exist — the refusal is in + front of this path rather than on it, which is the stronger place for it. + Asserted at creation, so that a change moving the check later would be + caught here rather than becoming a hang in the daemon. *) + (match Session.create ~file:"programs/pkg-macro-ring.flan" () with + | _ -> fail "a session over a ring of package macros was created" + | exception Loc.Error { Loc.dmsg = m; _ } -> + if not (has m "call each other") then + fail "creating a session over a macro ring said %S" m); + let ti, _ = Session.create ~file:"programs/pkg-macro-idle.flan" () in + (match Session.eval_expr ~origin:"programs/pkg-macro-idle.flan" ti "(s/spin)" with + | _ -> fail "a macro that does not settle was accepted through C-x C-e" + | exception Loc.Error { Loc.dmsg = m; _ } -> + if not (has m "did not settle") then + fail "a macro that does not settle, through C-x C-e, said %S" m); + (* And the session is still usable afterwards, which is the property the + whole of this file is about: a refusal that killed it would wedge the + editor just as thoroughly as the hang it is preventing. *) + (match Session.eval_expr ~origin:"programs/pkg-macro-idle.flan" ti "(mac/twice 21)" with + | _ -> () + | exception Loc.Error { Loc.dmsg = m; _ } -> + fail "a session that refused a macro could not evaluate afterwards: %s" m); + (* A form typed into a file that is *imported as a package* has to be qualified the way the import qualified it, or it splices as a brand-new unrelated name: the evaluation reports success and the running program