From 2d1d88e9d1dfedd391b066cfe1a1905e37103c2b Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Mon, 21 Sep 2026 16:00:49 +0700 Subject: [PATCH] A macro has a body, and the entry that said otherwise Review follow-ups on the mode pass. The regression first, because it is the one that cost something: giving macros a kind of their own took them out of three completion tables. flan-disassemble, flan-disassemble-ir and flan-lowering each filtered flan--defs on kind "fn", which had been the whole truth right up until this branch made it half of one. A macro is compiled -- defmacro is a defn by the time anything emits code -- so all three genuinely work on one, and only the offer had gone. One flan--compiled-kinds names both words and the three sites read it. Then four sentences in the FIX.org entry that were not true, which matters more than it sounds: that file is the history somebody reads later to find out what happened. The corpus claim was the bad one. It said the whole corpus round-trips with zero differing lines. It does not, and never did -- the script that measured it bound inhibit-message around its own reporting, which in batch means the differences were found and then swallowed. Measured properly: 317 files, 22 files and 389 lines differing before, 20 and 373 after. Sixteen lines fixed in two files, no new difference introduced, and 373 lines still differing that this pass never looked at. The other three: the fns/macros dedup is required by the new rows and is not a fix to a bug that was there before -- a macro used to appear once, as a fn. handler-case was already a keyword. loops.flan has three labelled loops and dotimes-range.flan has the other two. And three gaps the review found while checking: array-fill and array-gen were never in the keyword list, Unit was in the type rule while the parser refuses the word, and a prelude macro's row carried a bare name where a program's carried its parameters. A check that pulls every head out of parse.ml and diffs it against the three lists now comes back empty, which is what the docstring had started claiming. --- FIX.org | 55 +++++++++++++++++++++++++++++++------- emacs/flan-lower.el | 3 +-- emacs/flan-mode.el | 15 +++++++---- emacs/flan.el | 20 +++++++++++--- emacs/test-flan-mode.el | 30 +++++++++++++++++++++ emacs/test-flan.el | 5 ++++ lib/dev.ml | 59 ++++++++++++++++++++++++----------------- 7 files changed, 143 insertions(+), 44 deletions(-) diff --git a/FIX.org b/FIX.org index c7468ba6..58259fdc 100644 --- a/FIX.org +++ b/FIX.org @@ -6107,11 +6107,20 @@ report beside it. - [flan--constants] is [true false nil None context/allocator context/temp] and [uninit], matched as bare symbols. None of these is ever a head, so the paren-anchored rule never saw one and they were undrawn. -- [cast] and [none] were in the list and are not in the language at all. +- [cast] and [none] were in the list and are not in the language at all, and + [array-fill] and [array-gen] were missing from it — the parser reads both + itself, because their dimensions are in brackets and a bracket in expression + position would otherwise be an array literal. With those two added, every + head [Parse] dispatches on is now in one of the three lists or in one of the + two groups the docstring names as deliberately absent; a check that pulls the + heads out of parse.ml and diffs them against the lists comes back empty. - The type rule had no [dyn], [Allocator], [Vec], [Map], [Fn], [int] or [float], and no type variables — a generic defn's [$t] and the - [{:where ...}] clause over it were undrawn. -- [handler-case] is implemented now and was not drawn as a keyword. + [{:where ...}] clause over it were undrawn. [Unit] came off it: the resolver + answers to the name because [Cimport] builds one for C's void, but + [Parse.texpr] refuses the word outright, so drawing it advertised a spelling + that does not compile — the same rule that keeps [await] out of the keyword + list. - The syntax table did not know [$] or [&] were name characters, or that a comma is whitespace — [lib/reader.ml] skips one wherever a space would go. [&] matters more than it looks: a macro's parameter list destructures, so @@ -6126,13 +6135,28 @@ report beside it. - **A labelled loop indented its body under its own binding vector.** A label is a keyword written where the vector or the test goes, so it pushes the special arguments along by one; dotimes, while and until take one and loop - refuses one. test/programs/loops.flan has four. + refuses one. There are five in the corpus: three in test/programs/loops.flan + (lines 53, 60, 68) and two in dotimes-range.flan (99, 105). - [fn], [signal] and [error] had no indent entry. The [dotimes] arities needed none — the vector is one sexp whatever is inside it — and neither did the four object forms, which reach the [\`def] fallback. All six are pinned. -The whole corpus — test/programs/*.flan, examples/*.flan, vendor/*/*.flan and -sand.flan — round-trips through indent-region with zero differing lines. +Measured over every .flan file in the tree — 317 of them — by reindenting each +and counting the lines that came back different. Before this pass: 22 files, +389 lines. After: 20 files, 373 lines. So it fixes 16 lines in two files +(loops.flan 9, dotimes-range.flan 7, both of them labelled loops) and +introduces no new difference anywhere. + +The 20 files and 373 lines that still differ are untouched by this pass and +were untouched before it. They are concentrated in json.flan (85), +arena-region.flan (71), vendor/edn/provide.flan (55) and vendor/json (41), +and they are a separate piece of work: the indenter and the hand-formatting in +those files disagree about shapes nothing here looked at. + +An earlier version of this entry claimed zero differing lines. That was a bug +in the measurement, not a result: the script bound [inhibit-message] around +the loop that reported, which silences [message] in batch, so it printed +nothing and the nothing was read as a pass. ** CIDER-style dynamic fontification @@ -6144,15 +6168,28 @@ infer one from: [Parse] desugars [(defmacro m [a] ...)] into a defn, so every macro was already on the wire as a function and indistinguishable from one. So the op was extended rather than the editor made to guess. -- [macro], off Session.macros and off the prelude's. A macro is listed once - now: the fns list drops any name the macro set holds, which was also a - duplicate-row bug that predates this and made [assoc] answer [fn]. +- [macro], off Session.macros and off the prelude's. The fns list drops any + name the macro set holds, and that is required by the new rows rather than + a fix to anything: before this, a macro appeared exactly once, as kind [fn], + because the macro rows did not exist. Adding them without the filter is what + would have made [assoc] answer [fn] for a macro. The dropped fn's location is kept and put on the macro entry, so M-. on a macro still goes there and M-. on a prelude macro still refuses by naming the prelude rather than by shrugging about the daemon. + A macro row also has to be admitted wherever an editor asks for "a name with + a body": [flan-disassemble], [flan-disassemble-ir] and [flan-lowering] all + filtered their completion table on kind [fn], so giving macros a kind of + their own dropped them out of three lists. One [flan--compiled-kinds] now + names both words and all three read it. - [struct], [data], [union], [enum], [alias], read off the checker's environment — an enum and an alias are both gone from Tast.program by then. +A prelude macro carries its parameter vector like a program's, built through +the same function, so eldoc on [unless] shows [unless [& args] Form] and not +the bare word. The forms are held in a [lazy] here rather than fetched per +request: [Macro.prelude_macros] keeps only the names, and [Prelude.forms] +re-reads and re-parses the whole prelude every time it is called. + The editor side is a matcher function over a hash table, not a regexp: a regexp of every name would be rebuilt per refresh and, at a few thousand names, would eventually hit Emacs's regexp size limit. The table is built when diff --git a/emacs/flan-lower.el b/emacs/flan-lower.el index cbfa7eca..6dac6d37 100644 --- a/emacs/flan-lower.el +++ b/emacs/flan-lower.el @@ -615,8 +615,7 @@ was being read open." (list (or (thing-at-point 'symbol t) (completing-read "Lowerings of: " - (mapcar #'car (seq-filter (lambda (d) (equal (nth 1 d) "fn")) - flan--defs)) + (flan--compiled-names) nil nil nil nil (and (fboundp 'flan-current-defun-name) (flan-current-defun-name)))) diff --git a/emacs/flan-mode.el b/emacs/flan-mode.el index 3ec4d51f..86562657 100644 --- a/emacs/flan-mode.el +++ b/emacs/flan-mode.el @@ -129,7 +129,7 @@ (defconst flan--special '("quote" "do" "let" "if" "when" "cond" "and" "or" "while" "until" "break" "continue" "return" "set" - "array" "match" "fn" "dotimes" "loop" "recur" + "array" "array-fill" "array-gen" "match" "fn" "dotimes" "loop" "recur" "defer" "some" "try" "signal" "error" "handler-bind" "handler-case" "restart-case" "invoke-restart") "The heads `Parse.form' dispatches on — the forms with a meaning of their own. @@ -231,10 +231,15 @@ reason and is the odd one — it is legal only as the last item of a `def' or a ;; `Types.primitive_names', plus the four applied ones the checker ;; resolves and the function type. `dyn' is lowercase on purpose — it is ;; a primitive beside `i64' and `bool', not a container over something. - ;; `int' and `float' are builtin aliases for `i32' and `f32'. `Unit' is - ;; here because the resolver still answers to it, though nobody writes it: - ;; unit is spelled `()' and the parser refuses the word. - ("\\_<\\(?:[iu]\\(?:8\\|16\\|32\\|64\\)\\|f\\(?:32\\|64\\)\\|bool\\|string\\|dyn\\|int\\|float\\|Unit\\|Never\\|Allocator\\|Ptr\\|Option\\|Vec\\|Map\\|Fn\\)\\_>" + ;; `int' and `float' are builtin aliases for `i32' and `f32'. + ;; + ;; `Unit' is deliberately absent, though `Types.primitive_names' has it. + ;; The resolver answers to the name because `Cimport' builds one for C's + ;; void, but nothing anyone writes reaches that: `Parse.texpr' refuses the + ;; word outright — unit is spelled `()'. Drawing it as a valid type would + ;; advertise a spelling the parser rejects, which is the same reason + ;; `find-restart' and `await' are left out of `flan--special'. + ("\\_<\\(?:[iu]\\(?:8\\|16\\|32\\|64\\)\\|f\\(?:32\\|64\\)\\|bool\\|string\\|dyn\\|int\\|float\\|Never\\|Allocator\\|Ptr\\|Option\\|Vec\\|Map\\|Fn\\)\\_>" . font-lock-type-face) ;; A type variable, `$t', which is what a generic `defn' names its ;; parameter types with and what `{:where (ordered? $t)}' constrains. diff --git a/emacs/flan.el b/emacs/flan.el index 24ae741c..9efff105 100644 --- a/emacs/flan.el +++ b/emacs/flan.el @@ -1981,6 +1981,20 @@ Called for its effect on one buffer; `flan--dynamic-sync' does every buffer." (message "flan: %d names" (length flan--defs))) flan--defs) +(defconst flan--compiled-kinds '("fn" "macro") + "The kinds that have a body the compiler emitted code for. +A macro is one: `Parse' desugars `(defmacro m [a] …)' into a `defn', so it is +compiled, installed and disassemblable exactly as a function is. It reaches +this end as kind `macro' rather than as `fn' — that is the point of the kind +— and every list that offers \"a thing with a body\" has to say both words or +it silently stops offering macros.") + +(defun flan--compiled-names () + "Every name the program has a compiled body for, for a completion table." + (mapcar #'car + (seq-filter (lambda (d) (member (nth 1 d) flan--compiled-kinds)) + flan--defs))) + (defun flan--lookup (name) "The entry for NAME, or nil. @@ -2800,8 +2814,7 @@ tail of exactly one packaged name, because a buffer inside a package writes (list (or (thing-at-point 'symbol t) (completing-read "Disassemble: " - (mapcar #'car (seq-filter (lambda (d) (equal (nth 1 d) "fn")) - flan--defs)) + (flan--compiled-names) nil t nil nil (and (fboundp 'flan-current-defun-name) (flan-current-defun-name)))) @@ -2856,8 +2869,7 @@ that the IR half is findable by name rather than only by a modifier." (list (or (thing-at-point 'symbol t) (completing-read "LLVM IR for: " - (mapcar #'car (seq-filter (lambda (d) (equal (nth 1 d) "fn")) - flan--defs)) + (flan--compiled-names) nil t)))) (flan-disassemble name t)) diff --git a/emacs/test-flan-mode.el b/emacs/test-flan-mode.el index d6b37271..bdcfd7f1 100644 --- a/emacs/test-flan-mode.el +++ b/emacs/test-flan-mode.el @@ -400,6 +400,13 @@ font-lock-keyword-face "signal") ("(dotimes [b 3] (break :outer))" "break" font-lock-keyword-face "break") + ;; The two array constructors, which the parser reads itself + ;; because their dimensions are in brackets — see + ;; `test/programs/array-fill.flan'. + ("(array-fill [rows cols] 255)" "array-fill" + font-lock-keyword-face "array-fill") + ("(array-gen [n] f)" "array-gen" + font-lock-keyword-face "array-gen") ;; A builtin is a call, not a form the parser knows, and it is ;; drawn as one. ("(push v 1)" "push" font-lock-builtin-face "a builtin") @@ -432,6 +439,13 @@ (eq (test-flan-mode--face-at (nth 0 case) (nth 1 case)) (nth 2 case)))) +;; `Unit' is a name the resolver answers to and the parser refuses — unit is +;; written `()'. Drawing it as a type would advertise a spelling that does not +;; compile, which is the same rule that keeps `await' out of the keyword list. +(test-flan--check + "Unit is not drawn as a type, because the parser refuses the word" + (null (test-flan-mode--face-at "(defn f [] Unit 1)" "Unit"))) + ;; `context/allocator' has a slash in it and is not a qualified name: there is ;; no package called `context'. The constants rule runs first for exactly this ;; reason, and this is what would notice if it stopped. @@ -512,6 +526,22 @@ (not (eq (test-flan-mode--dyn-face "(with-retry (go))" "with-retry") (test-flan-mode--dyn-face "(settle 1 2)" "settle")))) +;; A macro is compiled, so it has a body to disassemble and to lower — and the +;; lists that offer "a thing with a body" used to filter on kind `fn' alone. +;; Giving macros a kind of their own would have quietly dropped them out of +;; `flan-disassemble', `flan-disassemble-ir' and `flan-lowering' unless every +;; one of those learned the second word. +(test-flan--check + "a macro is offered by the completion table for a compiled body" + (let ((flan--defs test-flan-mode--defs)) + (and (member "with-retry" (flan--compiled-names)) + (member "settle" (flan--compiled-names))))) + +(test-flan--check + "and a global is not, because it has no body" + (let ((flan--defs test-flan-mode--defs)) + (not (member "ticks" (flan--compiled-names))))) + ;; A constructor is `Type.Case' and is one symbol, so the type half is what ;; the program can speak for and the case half is left alone. (test-flan--check diff --git a/emacs/test-flan.el b/emacs/test-flan.el index f20fcc7f..a346716b 100644 --- a/emacs/test-flan.el +++ b/emacs/test-flan.el @@ -486,6 +486,11 @@ already rely on it — so nothing here is a stand-in for the real thing." (test-flan--check "and carries where it was written, so M-. still goes there" (let ((d (assoc "twice" flan--defs))) (and d (not (equal (nth 3 d) ""))))) + ;; A prelude macro carries its parameter vector like any other, so eldoc on + ;; `unless' shows a signature rather than the bare word. + (test-flan--check "a prelude macro carries a real signature" + (let ((d (assoc "unless" flan--defs))) + (and d (string-match-p "\\[.*\\] Form" (nth 2 d))))) ;; The prelude's, which must keep refusing for the reason it always did: ;; it is a string inside the compiler and not a file anyone can open. With ;; no location it would refuse for the wrong reason instead. diff --git a/lib/dev.ml b/lib/dev.ml index 316431d5..1a64e9c0 100644 --- a/lib/dev.ml +++ b/lib/dev.ml @@ -1413,6 +1413,15 @@ let signature_of_fn (f : Tast.fn) = editor already knows how to refuse. *) let macro_loc tbl name = try Hashtbl.find tbl name with Not_found -> "" +(* The prelude's [defmacro] forms, read once. [Macro.prelude_macros] holds only + the names and [defs] wants the parameter vectors too, so the forms are kept + here — and kept lazily for the reason that one is: [Prelude.forms] re-reads + and re-parses the whole prelude on every call, and [defs] is asked on connect + and again after every accepted evaluation. *) +let prelude_macro_forms = + lazy + (List.filter (fun f -> Macro.macro_name f <> None) (Prelude.forms ())) + let entry ~name ~kind ~sign ~loc ?(doc = "") () = Wire.list [ Wire.quote name; Wire.quote kind; Wire.quote sign; Wire.quote loc; @@ -1485,24 +1494,22 @@ let defs t = The signature is the parameter vector as written. A macro takes one parameter, the slice of argument forms, and answers a [Form]; see [Parse]'s [defmacro] arm, which desugars exactly that. *) - let macros = - List.filter_map - (fun (f : Form.t) -> - match Macro.macro_name f with - | None -> None - | Some name -> - let params = - match f.Form.v with - | Form.List (_ :: _ :: { Form.v = Form.Vec ps; _ } :: _) -> - String.concat " " (List.map Form.to_source ps) - | _ -> "" - in - Some - (entry ~name ~kind:"macro" - ~sign:(Printf.sprintf "%s [%s] Form" name params) - ~loc:(macro_loc macro_locs name) ())) - t.session.Session.macros + let macro_entry (f : Form.t) = + match Macro.macro_name f with + | None -> None + | Some name -> + let params = + match f.Form.v with + | Form.List (_ :: _ :: { Form.v = Form.Vec ps; _ } :: _) -> + String.concat " " (List.map Form.to_source ps) + | _ -> "" + in + Some + (entry ~name ~kind:"macro" + ~sign:(Printf.sprintf "%s [%s] Form" name params) + ~loc:(macro_loc macro_locs name) ()) in + let macros = List.filter_map macro_entry t.session.Session.macros in (* The prelude's, which the session's list deliberately does not hold — see [Macro.loaded_for], which drops them from a file's own set so that a prelude macro is not declared twice. [Session.macroexpand] can expand a @@ -1511,16 +1518,20 @@ let defs t = let prelude_macros = let own = List.filter_map Macro.macro_name t.session.Session.macros in List.filter_map - (fun name -> + (fun f -> + match Macro.macro_name f with (* A file may write its own [unless]; the session's entry above is then the one that answers, and listing this one after it would be a second row for one name. *) - if List.mem name own then None - else - Some - (entry ~name ~kind:"macro" ~sign:name - ~loc:(macro_loc macro_locs name) ())) - (Lazy.force Macro.prelude_macros) + | Some name when List.mem name own -> None + (* Built through the same [macro_entry] as the session's, so a prelude + macro carries its parameter vector too — eldoc on [unless] shows + [unless [& args] Form] rather than the bare word. + [Macro.prelude_macros] is only the names, so the forms are kept + beside it — see [prelude_macro_forms]. *) + | Some _ -> macro_entry f + | None -> None) + (Lazy.force prelude_macro_forms) in (* The type names, which an editor wants for the same reason it wants the function names: a struct the program defines is not an unknown word, and