diff --git a/FIX.org b/FIX.org index 35c940a2..ef87e108 100644 --- a/FIX.org +++ b/FIX.org @@ -6353,3 +6353,117 @@ out on ~program_asked~, so a re-run installs what is queued and then enters ~main~. That is ~lib/dev.ml~, which another lane holds, so it is written down here rather than done. The test row asserts the behaviour as it is, with the extra re-run spelled out. + +* An error in a macro's body is reported where it was written, 2026-09-21 +The author, on a falling-sand program: + +#+begin_quote +sand.flan:45:7: + takes two arguments or more, given 1 +#+end_quote + +Line 45 is a ~do-grid~ call. The mistake — ~(+ mr)~ with one operand — is on +line 49, inside the body the author passed to it. Every node of an expansion +carried the call site's location, so every error inside one landed on the call +and the ~^^^^~ underlined a macro that was perfectly fine. A macro that wraps +a twenty-line body made twenty lines of code report at one of them. + +** What a Form carries, and what it does not +A ~Form~ crosses to the compiled macro as 24 bytes: a tag and two payload +words. It mirrors ~Form.value~, not ~Form.t~, so there is no ~loc~ field, and +there is not going to be one — the padding hole at offset 4 belongs to the +queued structured-error work and nothing here touches the layout, the prelude's +~defdata Form~, or ~lib/form.ml~. + +What a Form does carry, for every case but ~Int~, ~Float~ and ~Byte~, is a +pointer: a name's or a string's bytes, or a bracket's children. This compiler +allocated that memory. And when a macro splices an argument through — which is +what ~~@body~ is — the 24-byte struct is copied but the pointer inside it is +not. The address is the part that survives the trip. + +** SBCL, read rather than recalled +~src/compiler/ir1tran.lisp~ has two mechanisms for this and they are not +equally good. ~*source-paths*~ (line 38) is an EQ hash table from cons to +source path, filled by ~find-source-paths~ walking a form before conversion; a +macro that splices returns the same conses, so the lookup still answers. +Nothing is stored on the object and identity is the whole key. +~recover-source-paths~ (line 918) is the fallback for macros that copy instead +of splice: a structural EQUAL match between the original and the expansion. Its +own comment admits false matches on duplicates and reordering, and SBCL gates it +on ~debug > 1~. + +The first is what this is. ~Expand.write~ records the payload pointer of each +node against its ~Loc.t~ in a table that lives for one call; ~Expand.unmarshal~ +looks each node's pointer up. A hit is a form the author wrote and it keeps its +own line. The second mechanism is not built and is not wanted: a guess that is +usually right is worse than a miss, because a location nobody can distrust is +the only kind worth printing. + +** What a miss does +It inherits — from the nearest enclosing node that resolved, and not from the +call site. Only a node with no located ancestor at all falls back to the call, +and that means its whole subtree was the macro's invention, which is exactly +when the call is the right place. + +~Int~, ~Float~ and ~Byte~ always miss, because their payload is the value and +there is no pointer to ask about. So a bad ~5~ in ~(foo (bar 5))~ is reported +at ~(bar 5)~, the smallest thing around it that has a place. SBCL draws the +same line for the same reason: ~source-form-has-path-p~ excludes symbols, +fixnums and characters. + +Pointer reuse cannot confuse it. Nothing ~Dynload~ hands out is freed until +~Dynload.release~ runs between the rounds in ~Macro~ or on the way out of an +expansion, so no address recorded during a call can be handed to a second +allocation while the table still holds it — and the table is one call's and is +dropped when the call returns either way. A macro that stashes a form in a +global and answers with it on a later call gets a miss, which is the harmless +direction. + +** Two rounds +~settle~ re-expands until the head stops being a macro. A form that survives +two rounds is written out a second time, and what is recorded the second time +is the ~Loc.t~ it came back with — its own, recovered in round one. So it keeps +the line the author wrote it on and not the intermediate call. +~Loc.from_macro~ is outermost-wins, so the ~note:~ names the macro the author +called and not the one it expanded into. + +** One addition beyond the brief: Loc.msite +~Loc.t~ had ~macro~ and nothing else, and ~Loc.diag~ pointed the "expanded from +the macro X" note at the diagnostic's own location. That was right only while +the two were the same place. The moment a spliced form keeps its own line, the +error is at line 49 and the call is at line 45, and one field cannot be both — +the note would have said "expanded from the macro do-grid" while pointing at a +line with no ~do-grid~ on it. + +So ~Loc.t~ gains ~msite : t option~, the call site for a location that is not +itself the call, set where the macro name is stamped and read by ~Loc.diag~. +Empty means this position *is* the call site, which is what every location that +has never been moved says, so nothing else changed. This is ~lib/loc.ml~ and +not the Form ABI. + +The report now reads, verbatim from the pinned test, which is the sand.flan +shape cut down to a file the suite can compile: + +#+begin_example + programs/macro-loc-body.flan:18:15: + takes two arguments or more, given 1 + 18 | (let [v (+ mr)] + | ^^^^^^ + programs/macro-loc-body.flan:17:5: note: expanded from the macro do-grid + 17 | (do-grid [br rows bc cols] + | -------------------------- +#+end_example + +** Pinned +Four programs under ~test/programs/macro-loc-*.flan~, each asserted on the +rendered report rather than on the location record — the line, the squiggle +under the right form and the note pointing at the call read as one sentence and +are three fields here. The spliced body several lines below its call; the +literal that takes the form around it; the node a macro built against the +leaves it built it from; and a body through two rounds of expansion. The +session path, which the editor reads, has a row of its own in +~test/test_session.ml~. + +No existing assertion moved. Every test in the tree that pins a diagnostic +location on a macro call pins one raised *before* expansion — the four +parameter-list refusals in ~Expand.check_call~ — or one on a node the macro +built, and both still report at the call. The suite was green on the first run +after the change. diff --git a/lib/check.ml b/lib/check.ml index 5d05d087..cc2d6e39 100644 --- a/lib/check.ml +++ b/lib/check.ml @@ -7908,10 +7908,10 @@ and named_call ?(qualified = false) ctx ~want loc name args = in a *data* file, which no symbol the expansion could invent will hold. So a macro that has to refuse expands to a call to this, and the string is - the report. [Loc.from_macro] has already stamped the call site onto every - node of the expansion, so the location is the [defedn] the author wrote - and the sentence is the macro's — which is the two halves the prelude's - note says are never both right at once. + the report. The string is a form the macro built, so it has no line of its + own and takes the call site's — the [defedn] the author wrote. The + location is theirs and the sentence is the macro's, which is the two + halves the prelude's note says are never both right at once. A builtin and not a declaration, because it has to fail *here*: a declared function would compile, link and run, and the compile it was meant to stop diff --git a/lib/expand.ml b/lib/expand.ml index 15f74efb..c058f339 100644 --- a/lib/expand.ml +++ b/lib/expand.ml @@ -57,20 +57,44 @@ let tag_of_int = function dynload_stubs.c, one field at a time. Everything allocated here is owned by [Dynload] and released together after the call. *) +(* ── Which node a pointer belongs to ─────────────────────────────── + A [Form] on the wire has no [loc] field and is not getting one. What it does + have, for every case but the three that fit inside the payload, is a + pointer: a string's bytes, or a bracket's children. This compiler allocated + that memory, so the address names the node it was written for — and when a + macro splices one of its arguments through untouched, the 24-byte struct is + copied but the pointer inside it is not. The address is the part that + survives the trip, so it is what a form coming back out is recognised by. + + That is SBCL's [*source-paths*] (src/compiler/ir1tran.lisp), an EQ table + from the conses of a form to where they were read, which still answers after + a macro splices those same conses into its expansion. The address stands in + for [eq] because across a C ABI nothing else can: two sides that share no + heap share no notion of identity but the pointer. + + One table per call, made in [call] and gone when it returns. A macro that + keeps a form from one call and answers with it in another gets a miss, and a + miss is the harmless direction. *) + +type sites = (Dynload.addr, Loc.t) Hashtbl.t + (* Into an existing 24 bytes, which is what an argument array needs: the macro takes a [Form] slice, and a slice is contiguous elements and not an array of pointers — so this writes *into* memory the caller took, and every caller here takes it as part of an array. *) -let rec write p (f : Form.t) = +let rec write (sites : sites) p (f : Form.t) = let tag t = Dynload.poke_i32 p 0 (tag_int t) in + let note b = Hashtbl.replace sites b f.Form.loc in let str t s = tag t; let n = String.length s in (* A zero-length string still gets a pointer, because a slice with a null base is not the same value as one with a live base and a zero length -- - the difference shows the day something concatenates onto it. *) + the difference shows the day something concatenates onto it. It also + keeps every node's key distinct, which the table above depends on. *) let b = Dynload.take (max n 1) in if n > 0 then Dynload.poke_bytes b 0 s; + note b; Dynload.poke_ptr p ptr_off b; Dynload.poke_i64 p len_off (Int64.of_int n) in @@ -78,7 +102,10 @@ let rec write p (f : Form.t) = tag t; let n = List.length xs in let b = Dynload.take (max (n * form_size) 1) in - List.iteri (fun i x -> write (Nativeint.add b (Nativeint.of_int (i * form_size))) x) xs; + List.iteri + (fun i x -> write sites (Nativeint.add b (Nativeint.of_int (i * form_size))) x) + xs; + note b; Dynload.poke_ptr p ptr_off b; Dynload.poke_i64 p len_off (Int64.of_int n) in @@ -94,37 +121,59 @@ let rec write p (f : Form.t) = | Form.Map xs -> seq TMap xs (* ── Reading one back ────────────────────────────────────────────── - [loc] is the call site's, stamped onto every node. A macro cannot invent a - source location and the image has no room for one: Form on the Flan side - mirrors [Form.value], not [Form.t]. So an error inside an expansion points - at the call that produced it, which is the part of "the error carries the - expansion" that can be had now without the structured-error rewrite. *) + Three ways a node gets a location, and which one applies is decided by the + table above. -let rec unmarshal ~loc (p : Dynload.addr) : Form.t = - let str () = - let b = Dynload.peek_ptr p ptr_off in - let n = Int64.to_int (Dynload.peek_i64 p len_off) in - if n = 0 then "" else Dynload.peek_bytes b 0 n + A hit is a form the author wrote: it went in on some line, the macro passed + it through, and it comes back with the pointer it went in with. It keeps its + own line, tagged with the macro whose call it was written inside — so the + error is reported where the code is and the [note:] still says which call + put it there. + + A miss is a node the macro built, and it has no line of its own anywhere. It + takes the nearest enclosing node that did have one, which is the smallest + piece of what the author wrote that contains it. Only a node with no located + ancestor at all falls back to the call site, and that means the whole + subtree was the macro's invention. + + [Int], [Float] and [Byte] are always a miss, because their payload is the + value and there is no pointer to ask about — reading one as an address would + be a number pretending to be a node. So a bad [5] in [(foo (bar 5))] is + reported at [(bar 5)], the nearest thing that has a place. That is the same + line SBCL draws: its [source-form-has-path-p] excludes symbols, fixnums and + characters for the same reason. *) + +let rec unmarshal ~(sites : sites) ~loc (p : Dynload.addr) : Form.t = + let ptr () = Dynload.peek_ptr p ptr_off in + let len () = Int64.to_int (Dynload.peek_i64 p len_off) in + let str () = let n = len () in if n = 0 then "" else Dynload.peek_bytes (ptr ()) 0 n in + (* The node's own location, or the one it inherits. [Loc.from_macro] is + outermost-wins and the call site is already tagged, so a form that came + through two expansions keeps the name of the macro the author wrote. *) + let here () = + match Hashtbl.find_opt sites (ptr ()) with + | None -> loc + | Some own -> + (match loc.Loc.macro with + | None -> own + | Some m -> Loc.from_macro ~at:(Loc.call_site loc) m own) in - let seq () = - let b = Dynload.peek_ptr p ptr_off in - let n = Int64.to_int (Dynload.peek_i64 p len_off) in + let seq loc = + let b = ptr () and n = len () in List.init n (fun i -> - unmarshal ~loc (Nativeint.add b (Nativeint.of_int (i * form_size)))) + unmarshal ~sites ~loc (Nativeint.add b (Nativeint.of_int (i * form_size)))) in - let v = - match tag_of_int (Dynload.peek_i32 p 0) with - | TSym -> Form.Sym (str ()) - | TKw -> Form.Kw (str ()) - | TStr -> Form.Str (str ()) - | TInt -> Form.Int (Dynload.peek_i64 p payload) - | TFloat -> Form.Float (Dynload.peek_f64 p payload) - | TByte -> Form.Byte (Int32.to_int (Dynload.peek_i32 p payload) land 0xff) - | TList -> Form.List (seq ()) - | TVec -> Form.Vec (seq ()) - | TMap -> Form.Map (seq ()) - in - Form.make v loc + match tag_of_int (Dynload.peek_i32 p 0) with + | TInt -> Form.make (Form.Int (Dynload.peek_i64 p payload)) loc + | TFloat -> Form.make (Form.Float (Dynload.peek_f64 p payload)) loc + | TByte -> + Form.make (Form.Byte (Int32.to_int (Dynload.peek_i32 p payload) land 0xff)) loc + | TSym -> Form.make (Form.Sym (str ())) (here ()) + | TKw -> Form.make (Form.Kw (str ())) (here ()) + | TStr -> Form.make (Form.Str (str ())) (here ()) + | TList -> let l = here () in Form.make (Form.List (seq l)) l + | TVec -> let l = here () in Form.make (Form.Vec (seq l)) l + | TMap -> let l = here () in Form.make (Form.Map (seq l)) l (* ── One call ────────────────────────────────────────────────────── The arguments are one contiguous run of Forms, not an array of pointers, @@ -133,13 +182,19 @@ let rec unmarshal ~loc (p : Dynload.addr) : Form.t = let call ~loc (fn : Dynload.addr) (args : Form.t list) : Form.t = let n = List.length args in + (* This call's, and no other's. Nothing [Dynload] hands out is freed before + the whole expansion is over — [Dynload.release] runs between the rounds in + [Macro] and on the way out of one — so no address recorded here can be + handed to a second allocation while the table still holds it, and the + table is dropped the moment this returns either way. *) + let sites : sites = Hashtbl.create 64 in let a = Dynload.take (max (n * form_size) 1) in List.iteri - (fun i x -> write (Nativeint.add a (Nativeint.of_int (i * form_size))) x) + (fun i x -> write sites (Nativeint.add a (Nativeint.of_int (i * form_size))) x) args; let out = Dynload.take form_size in Dynload.call fn a (Int64.of_int n) out; - unmarshal ~loc out + unmarshal ~sites ~loc out (* ── Quasiquote ──────────────────────────────────────────────────── A desugaring over [Form], and nothing more: a quasiquoted (if ~t ~b) becomes @@ -343,12 +398,11 @@ let params_of (v : Form.t) : msig = sg (* ── Checking a call against it ──────────────────────────────────── - Before expansion, so the location is the call's own and not the - [Loc.from_macro] stamp every node of an expansion carries. That is the whole - reason this is a separate pass rather than something the macro body could - do: a macro has no error facility, and by the time its body runs the only - location left is the one it was called from anyway — stamped onto forms the - author never wrote. *) + Before expansion, so the refusal is about the call as it is written and + points at it. That is the whole reason this is a separate pass rather than + something the macro body could do: a macro has no error facility, and a call + that does not fit the parameter list is a mistake in the call rather than in + anything the expansion would go on to produce. *) let written (sg : msig) = Form.to_string sg.src diff --git a/lib/loc.ml b/lib/loc.ml index e5bacb24..b99294d8 100644 --- a/lib/loc.ml +++ b/lib/loc.ml @@ -22,25 +22,42 @@ type t = { ecol : int; (* The macro whose expansion produced whatever is at this position, if one did. It rides on the location rather than on the form because the location - is the thing that already travels: [Expand.unmarshal] stamps the call - site onto every node a macro answers with, and that stamp goes on through - the AST and the typed IR untouched. Tagging it here means an error raised - anywhere downstream can say which macro it is really about, with no field - added to Form, to Ast or to Tast. *) + is the thing that already travels: [Expand.unmarshal] puts this on every + node a macro answers with, and it goes on through the AST and the typed IR + untouched. Tagging it here means an error raised anywhere downstream can + say which macro it is really about, with no field added to Form, to Ast or + to Tast. *) macro : string option; + (* Where that macro was called, when this position is not the call itself. + The two came apart when [Expand.unmarshal] learned to give a spliced form + back the line the author wrote it on: the error is at the author's line + and the call is elsewhere, so one field cannot be both. Empty means this + position *is* the call site, which is what every location that has never + been moved says. *) + msite : t option; } let make file line col = - { file; line; col; eline = line; ecol = col; macro = None } + { file; line; col; eline = line; ecol = col; macro = None; msite = None } let unknown = make "" 0 0 (** Tag a location as coming out of [name]'s expansion, unless it already names a macro. Already-tagged wins because the tag is applied outermost-last: the macro the author actually wrote is the one worth naming, not whatever it - expanded into on the way. *) -let from_macro name (t : t) = - match t.macro with None -> { t with macro = Some name } | Some _ -> t + expanded into on the way. + + [at] is where the call was written, for a location that is somewhere else — + a form the author wrote inside the call and the macro passed through. Left + out, the location is the call site and says so by leaving [msite] empty. *) +let from_macro ?at name (t : t) = + match t.macro with + | Some _ -> t + | None -> { t with macro = Some name; msite = at } + +(** Where to point when saying which macro a position came out of: the call + site if this position is not it. *) +let call_site (t : t) = match t.msite with Some s -> s | None -> t (** [upto start stop] is [start] widened to end where [stop] begins. A [stop] that is not after [start], or is in another file, leaves it alone: a span @@ -85,12 +102,12 @@ let to_string t = Printf.sprintf "%s:%d:%d" t.file t.line t.col state one of the two, which is why volume of messages was never the whole of what was missing. - [expansion] names the macro call an error came from. A form a macro produced - carries the call site's location, so without this the report would point at - the call and say nothing about the code not being what was written there. It - is filled in for errors raised *during* expansion; a checker error on a form - a macro produced gets the call site's location without the macro's name, - which is a limitation and not a claim. *) + [expansion] names the macro call an error came from, and points at the call. + A form the author wrote inside a macro call keeps its own line through the + expansion, so the error is reported where it was written and this is the + second place the reader needs: the call that took the form and put it + somewhere it does not work. A form the macro built has no line of its own + and is reported at the call, where the two coincide. *) type severity = Info | Warning | Err @@ -147,7 +164,8 @@ let diag ?(kind = "error") ?(notes = []) ?expansion loc msg = let expansion = match expansion with | Some _ as e -> e - | None -> (match loc.macro with Some m -> Some (m, loc) | None -> None) + | None -> + (match loc.macro with Some m -> Some (m, call_site loc) | None -> None) in sort_notes { kind; dloc = loc; dmsg = msg; notes; expansion } diff --git a/lib/macro.ml b/lib/macro.ml index 6d5a3478..177b4c34 100644 --- a/lib/macro.ml +++ b/lib/macro.ml @@ -319,9 +319,10 @@ let compile (names : string list) (extra : Form.t list) : loaded = (* ── Where the call site is ──────────────────────────────────────── The one thing a macro cannot find out for itself and the one it needs to - read a data file: a Form carries no location — deliberately, see - [Expand.unmarshal] — so a macro handed [(defedn T "assets/x.edn")] knows the - path and not what it is relative to. [(embed "assets/x.edn")] resolves + read a data file: a Form on the wire carries no location — the compiler + keeps track of that on its own side, see [Expand.unmarshal] — so a macro + handed [(defedn T "assets/x.edn")] knows the path and not what it is + relative to. [(embed "assets/x.edn")] resolves against the directory of the source file the form is written in, and a macro reading a file has to resolve it the same way or a package's data would depend on where flan was invoked from. @@ -393,11 +394,12 @@ let rec expand_form (l : loaded) (f : Form.t) : Form.t = match f.Form.v with | Form.List ({ Form.v = Form.Sym n; _ } :: args) when List.mem_assoc n l.fns -> let args = List.map (expand_form l) args in - (* The call site, tagged with the macro it is a call to. [Expand.unmarshal] - stamps it onto every node the macro answers with, so from here down - every form it produced knows where it came from and an error on one of - them can say so. [checked_call] is where that tagging happens, along - with the arity and destructuring check that has to come first. *) + (* The call site, tagged with the macro it is a call to. It is what a node + the macro *built* is reported at — a form the author wrote inside the + call keeps its own line, which [Expand.unmarshal] recovers — and either + way the tag is what lets the report name the macro. [checked_call] is + where that tagging happens, along with the arity and destructuring + check that has to come first. *) settle l n loc (checked_call l n ~loc args) fuel | Form.List xs -> Form.make (Form.List (List.map (expand_form l) xs)) loc | Form.Vec xs -> Form.make (Form.Vec (List.map (expand_form l) xs)) loc @@ -570,8 +572,8 @@ let () = Parse.expander := program Both answer the name at the head when it is a macro, so the editor can say *which* macro it just ran rather than only that something changed. That name is the one thing the printed text cannot carry: [Loc.from_macro] is - outermost-wins, so every node of a full expansion is stamped with the macro - the author wrote and the intermediate names are gone by the time it settles. + outermost-wins, so a form the author wrote is tagged with the macro they + called and the intermediate names are gone by the time it settles. One step is outermost-only, and that is a deliberate difference from [expand_form], which expands a call's arguments *before* calling it. So diff --git a/lib/prelude.ml b/lib/prelude.ml index 0d89657a..f8c9c400 100644 --- a/lib/prelude.ml +++ b/lib/prelude.ml @@ -2056,11 +2056,13 @@ let source = {flan| ;; edited together. ;; ;; It mirrors Form.value and not Form.t: there is no `loc` field. A macro -;; cannot invent a source location and should not carry one, so the compiler -;; stamps the *call site's* location onto every node of what a macro returns. -;; That is the structural version of "keep the source location of the call -;; site attached to what a macro produces", and it is what the queued -;; structured-error work will read. +;; cannot invent a source location and should not carry one, so locations stay +;; on the compiler's side. It still knows where a form came from: every case +;; but the three that fit inside the payload holds a pointer into memory the +;; compiler allocated, and lib/expand.ml keeps a table from that address to the +;; line the form was read on. A form a macro splices through comes back holding +;; that pointer, so an error on it is reported where it was written; a node the +;; macro built is reported at the call. ;; ;; Case order is the tag order (docs/BUILT.md, data types), so this list is a layout ;; contract with lib/expand.ml's marshaller and may not be reordered. diff --git a/test/programs/macro-arity.flan b/test/programs/macro-arity.flan index 5b5d9945..57084986 100644 --- a/test/programs/macro-arity.flan +++ b/test/programs/macro-arity.flan @@ -2,8 +2,8 @@ ;;;; ;;;; Nothing here is a run-time claim and nothing here expands: check_call ;;;; counts the call against the list before do-grid is ever run, so the -;;;; location is the line below rather than a node of an expansion stamped -;;;; with Loc.from_macro. +;;;; location is the line below and there is no expansion to report anywhere +;;;; else. (defmacro do-grid [[r rows c cols] & body] `(dotimes [~r ~rows] (dotimes [~c ~cols] ~@body))) diff --git a/test/programs/macro-loc-body.flan b/test/programs/macro-loc-body.flan new file mode 100644 index 00000000..75ca1a26 --- /dev/null +++ b/test/programs/macro-loc-body.flan @@ -0,0 +1,20 @@ +;;;; A mistake in a body a macro spliced is reported where it was written. +;;;; +;;;; do-grid takes the body apart and puts it back inside two dotimes, and +;;;; nothing in it is rewritten on the way. So the (+ mr) below is the very +;;;; form the author wrote, and the line it is reported on is the one it is +;;;; written on -- not the line of the do-grid call, which is where every node +;;;; of an expansion used to land. +(defmacro do-grid [[r rows c cols] & body] + `(dotimes [~r ~rows] + (dotimes [~c ~cols] + ~@body))) + +(defn main [] i32 + (let [rows 4 + cols 4 + mr 1] + (do-grid [br rows bc cols] + (let [v (+ mr)] + (set v v)))) + 0) diff --git a/test/programs/macro-loc-literal.flan b/test/programs/macro-loc-literal.flan new file mode 100644 index 00000000..003e0e75 --- /dev/null +++ b/test/programs/macro-loc-literal.flan @@ -0,0 +1,19 @@ +;;;; A number a macro passed through is reported at the form around it. +;;;; +;;;; A Form on the wire carries a pointer for a name, a string and a bracket, +;;;; and that pointer is how a form that went into a macro is recognised when +;;;; it comes back out. An integer has no pointer -- its payload is the value +;;;; -- so the 5 below has nothing to be recognised by and takes the location +;;;; of the (g ...) around it, which does. So the error lands on the line the +;;;; call opens on rather than the line the number is on, and the two are kept +;;;; apart below so that the difference is visible. +(defmacro thru [& body] + `(do ~@body)) + +(defn g [s string] i32 0) + +(defn main [] i32 + (thru + (g + 5)) + 0) diff --git a/test/programs/macro-loc-nested.flan b/test/programs/macro-loc-nested.flan new file mode 100644 index 00000000..e60fb493 --- /dev/null +++ b/test/programs/macro-loc-nested.flan @@ -0,0 +1,18 @@ +;;;; Two rounds of expansion, and the body comes through both of them. +;;;; +;;;; outer expands into a call to inner, which expands again. The forms below +;;;; are written once and marshalled twice, so the second round has to find +;;;; the location the first round gave back rather than the call site it was +;;;; passing through. The note names outer, the macro that was written here, +;;;; and not inner, which nobody wrote. +(defmacro inner [& body] + `(do ~@body)) + +(defmacro outer [& body] + `(inner ~@body)) + +(defn main [] i32 + (outer + (let [a 1] + (+ a nowhere))) + 0) diff --git a/test/programs/macro-loc-rebuilt.flan b/test/programs/macro-loc-rebuilt.flan new file mode 100644 index 00000000..8081c5df --- /dev/null +++ b/test/programs/macro-loc-rebuilt.flan @@ -0,0 +1,17 @@ +;;;; A node the macro built has no line; the leaves it built it from keep +;;;; theirs. +;;;; +;;;; flip does not splice a body, it assembles a new list out of its two +;;;; arguments. That list is the macro's own -- it is reported at the call -- +;;;; while nosuchname inside it is the author's form, spliced through +;;;; untouched, and is reported where it is written. +(defmacro flip [a b] + `(~b ~a)) + +(defn g [x i32] i32 x) + +(defn main [] i32 + (flip + nosuchname + g) + 0) diff --git a/test/test_acceptance.ml b/test/test_acceptance.ml index 6de3de75..3cb833a1 100644 --- a/test/test_acceptance.ml +++ b/test/test_acceptance.ml @@ -4118,6 +4118,61 @@ level "1" print_endline "FAIL the report does not say which macro" end); + (* ── Where a mistake inside a macro call is reported ───────────── + A form the author writes inside a macro call is handed to the macro and + usually handed straight back, and it comes back holding the pointer it + went out with — the string's bytes, or a bracket's children, memory this + compiler allocated. [Expand] keeps a table of those addresses for the + length of one call, which is how a spliced form gets its own line back + instead of the call's. SBCL's [*source-paths*] does the same thing with + an EQ table over conses. + + Each row is asserted on the *rendered* report rather than on the + location record, because the three parts that have to line up — the + line, the squiggle under the right form, and the note pointing at the + call — read as one sentence and are three fields here. *) + let reports name path wanted = + match + let l = Load.program ~file:path (Reader.read_file path) in + ignore (Check.program l.Load.decls) + with + | () -> + incr failures; + Printf.printf "FAIL %s\n it was accepted\n" name + | exception Loc.Error d -> + let text = Loc.report d in + List.iter + (fun w -> + if not (contains text w) then begin + incr failures; + Printf.printf "FAIL %s\n said: %s\n wanted: %S in it\n" + name text w + end) + wanted + in + (* The falling-sand case this was written for: the body is several lines + below the call, and the call is what used to be reported. *) + reports "a mistake in a spliced body" "programs/macro-loc-body.flan" + [ "macro-loc-body.flan:18:15: + takes two arguments or more, given 1"; + "macro-loc-body.flan:17:5: note: expanded from the macro do-grid" ]; + (* A number has no pointer to be recognised by, so it takes the location of + the call around it — a form the author wrote, which does have one. *) + reports "a literal a macro passed through" "programs/macro-loc-literal.flan" + [ "macro-loc-literal.flan:17:5: expected string, found the integer \ + literal 5"; + "macro-loc-literal.flan:16:3: note: expanded from the macro thru" ]; + (* The list is the macro's and is reported at the call; the name inside it + is the author's and is reported where it is written. *) + reports "a node the macro built" "programs/macro-loc-rebuilt.flan" + [ "macro-loc-rebuilt.flan:15:5: unknown name nosuchname"; + "macro-loc-rebuilt.flan:14:3: note: expanded from the macro flip" ]; + (* Marshalled twice. The location survives both rounds, and the note names + the macro that was written rather than the one it expanded into. *) + reports "a body through two rounds of expansion" + "programs/macro-loc-nested.flan" + [ "macro-loc-nested.flan:17:12: unknown name nowhere"; + "macro-loc-nested.flan:15:3: note: expanded from the macro outer" ]; + (* Every prelude macro's arity guard, said the only way a macro can say anything: a call to a name nothing defines, reported at the call site. The point of pinning these is that without a guard the macro would @@ -4164,10 +4219,8 @@ level "1" "none can be compiled first"; (* A call that does not fit the macro's parameter list, in all four of the ways it can fail to. Every one of them is refused *before* the macro is - expanded, which is why each message carries the call's own location - rather than the [Loc.from_macro] stamp every node of an expansion gets — - that stamping is a documented limitation waiting on the structured-error - rewrite, and these four are the part of it that does not have to wait. *) + expanded, which is why each message is about the call as it is written + and points at it: there is no expansion yet to point anywhere else. *) refuses "a macro call with too few arguments" "programs/macro-arity.flan" "do-grid takes at least 1 argument and this call gives 0 — its \ parameter list is [[r rows c cols] & body], where &body is the rest"; diff --git a/test/test_session.ml b/test/test_session.ml index d3fbfd30..db1ffade 100644 --- a/test/test_session.ml +++ b/test/test_session.ml @@ -690,6 +690,37 @@ let () = | exception Loc.Error { Loc.dmsg = m; _ } -> fail "an edited defmacro: %s" m); + (* A mistake in a body the macro spliced, reported where it was written. + Same machinery as a build — [Macro.expand_form] and [Expand.call] — and + the point of asking it here is that the editor is where it is read: C-c + C-c on a form four lines long puts the cursor on the line the message + names, and naming the macro call would put it on the wrong one every + time. The body is on the third line of what is sent and the call is on + the first. *) + (match Session.eval ~origin:"programs/pkg-macro.flan" tm + "(defmacro splice [& body] `(do ~@body))" + with + | _ -> () + | exception Loc.Error { Loc.dmsg = m; _ } -> + fail "evaluating a splicing defmacro: %s" m); + (match + Session.eval ~origin:"programs/pkg-macro.flan" tm + "(defn spliced [] i32\n\ + \ (splice\n\ + \ (+ nowhere 1))\n\ + \ 0)" + with + | _ -> fail "a mistake in a spliced body was accepted" + | exception Loc.Error d -> + if d.Loc.dloc.Loc.line <> 3 then + fail "a mistake in a spliced body was reported on line %d, not 3: %s" + d.Loc.dloc.Loc.line d.Loc.dmsg; + (match d.Loc.expansion with + | Some ("splice", at) when at.Loc.line = 2 -> () + | Some (n, at) -> + fail "the note on a spliced body says %s at line %d" n at.Loc.line + | None -> fail "the note on a spliced body names no macro")); + (* The robustness lane's property, held for macros too: the commit is below the checker, so a [defmacro] that does not check leaves the session holding nothing of it. The body calls an unknown name, so the form parses