diff --git a/HANDOFF-macros.md b/HANDOFF-macros.md new file mode 100644 index 0000000..6422b30 --- /dev/null +++ b/HANDOFF-macros.md @@ -0,0 +1,76 @@ +# Handoff: macros in an imported package + +**Stopped mid-task when the session ended.** The feature works; the acceptance wiring is unfinished. +Its author's last words were "Works. Now the acceptance wiring." This file was written by the +coordinator from the diff and from running the program, not by the lane, so treat the "what remains" +list as read off the tree rather than as the author's own account. + +## State + +`dune build` succeeds. `test/programs/pkg-macro.flan` runs and prints `8 12 10 12 10 8 70 60`, +which is a package macro, a package macro that quasiquotes another, and a program macro, all +coexisting. **`dune test` was never run.** That is the first thing to do. + +## The shape picked, and why the refusal's reasoning was wrong + +The old refusal in `lib/load.ml` said collecting a package's macros would need that package's +imports resolved **at the Form level, before `Load` runs** — a second import resolver — and refused +rather than build one. + +The lane found the premise false, and this is the finding worth keeping: + +> the *file being compiled* is parsed before `Load` runs too, so no shape of the feature could have +> left import resolution where it was. + +So the phases moved instead of being duplicated. **`Load.program` takes forms now**: it reads the +import forms, resolves them with the one resolver it always had, and parses the file with the +packages' macros already in front of it. There is no second resolver. + +The acyclic-import guarantee is what makes this sound, and it was already there for this reason — +`load.ml`'s own comment says a definite package order is what the macro expander needs. + +## The rule + +Package macros arrive **qualified**, exactly as a `defn` does. A program importing the directory as +`mac` writes `(mac/twice 4)`; `(twice 4)` is an unknown name. Nothing becomes globally visible by +importing a package. Inside the package the names are unqualified, which is the rule every other +declaration there follows. + +## Files touched + +`bin/main.ml`, `lib/load.ml`, `lib/macro.ml`, `lib/parse.ml`, `lib/session.ml`, +`test/programs/pkg-macro.flan`, `test/programs/pkgs/mac/mac.flan`, `test/test_acceptance.ml`, +`test/test_reload.ml`, `test/test_sanitize.ml`, `test/test_session.ml`, `test/test_valgrind.ml`, +`test/test_web.ml`. 447 insertions, 117 deletions. + +The test files are mostly signature churn from `Load.program` taking forms — that call is made in +many places and each had to move. + +## What remains + +1. **Run `dune test --root .` and make it green.** Nothing has run it. `Load.program`'s signature + changed and it is called from most test binaries, so expect breakage that is mechanical rather + than deep. +2. **Finish the acceptance wiring.** `test_acceptance.ml:1423-1427` was the refusal case + (`refuses "a macro in an imported package"`). The diff touches it; confirm it now asserts the + working program and that a *sibling* refusal pins the qualified-name rule — that `(twice 4)` + unqualified is an unknown name. `pkg-macro.flan`'s header says that refusal exists; verify it + does. +3. **Check the non-termination refusals still fire** when the macros come from a package: a ring of + macros is named, a macro that does not settle is bounded. `BUILT.md` records both. Neither was + confirmed under the new path. +4. **Check the dev loop.** A macro imported by the file being edited must still be available on a + `C-c C-c`. `lib/session.ml` is in the diff, so this was at least considered, but nothing proves + it. `test/test_session.ml` and `test/test_dev.ml` show how sessions are driven. +5. **Measure the build cost** against the recorded baseline in `BUILT.md`: 50ms for a build naming + no macro, 310ms cold and 70ms warm for one calling a macro. Whether reading a package's forms + earlier moved those numbers is unknown. +6. **`vendor/raylib` is the customer.** `with-drawing` and `with-mode-2d` over raylib's + `BeginDrawing`/`EndDrawing` and `BeginMode2D`/`EndMode2D`, so an unbalanced pair stops being + possible. A raylib lane tried and could not; that is what prompted this work. Not started here + and deliberately out of scope — that file was held by another lane at the time. + +## Not verified + +Everything above item 1. The program runs and the build is clean; that is the whole of what is +known to work. diff --git a/bin/main.ml b/bin/main.ml index 5958151..5b91576 100644 --- a/bin/main.ml +++ b/bin/main.ml @@ -47,7 +47,7 @@ let summarise (d : Flan.Ast.decl) = asks for the whole list rather than the first thing wrong. *) let load path : Flan.Load.t = Flan.Load.program ~file:path - (Flan.Parse.program_all (Flan.Reader.read_file path)) + ~parse:Flan.Parse.program_all (Flan.Reader.read_file path) let checked path = Flan.Check.program_all (load path).decls diff --git a/lib/load.ml b/lib/load.ml index 6c5d7e9..9e690d6 100644 --- a/lib/load.ml +++ b/lib/load.ml @@ -46,6 +46,13 @@ type t = { a REPL editing a package's source needs it to know that [poll] typed in vendor/agent/agent.flan means [agent/poll] to the running program. *) pkgs : pkg list; + (* Every [defmacro] the imported packages declare, qualified under the alias + each was imported as and quasiquote-desugared, ready for + [Parse.imported_macros]. It is carried out of here rather than left behind + because the parse that needs it outlives the one this drove: a C-c C-c on + a function that calls [rl/with-drawing] is a fresh [Parse.program] with no + import form in sight, and [Session] hands this back to it. *) + macros : Form.t list; } (* [pcsrcs] and [plflags] are the package's own, kept per-package rather than @@ -63,6 +70,8 @@ and pkg = { alias : string; dir : string; owns : string list; import. See [Cimport]. *) phidden : (string * string) list } +let empty = { decls = []; csrcs = []; lflags = []; pkgs = []; macros = [] } + let fail loc fmt = Printf.ksprintf (fun m -> Loc.raise_diag (Loc.diag loc m)) fmt (* "vendor:raylib" -> the collection "vendor" and the subpath "raylib". A path @@ -325,6 +334,157 @@ let qualify_decl owned alias (d : Ast.decl) : Ast.decl = in { d with Ast.d = k } +(* ── Qualifying a package's macros ────────────────────────────────── + The rename above works over the Ast and a macro cannot go that way. By the + time [Parse] is finished with a [defmacro] its quasiquote has been desugared + into [form-cons] and [form-nil] calls, and a quasiquoted [(begin)] is a + [(Form.Sym {.s "begin"})] whose name is a *string in an argument* rather + than a name anything would rename. That is not an accident to work around: + it is the property [Macro]'s walk depends on, the one that makes a + quasiquoted call output rather than a compile-order dependency (BUILT.md, + "A call inside a quasiquote is output, not a dependency"). It is also + exactly what puts the name out of a rename's reach. + + So a package's macro is renamed here instead, over the text its author + wrote, where [`(begin)] and [(begin)] are still the same shape and one rule + covers both: a symbol the package owns becomes [alias/symbol]. The + quasiquote is desugared afterwards, so the walk still sees what it needs to. + + The importer's own forms are never at risk of being caught by this. They do + not appear in the text at all — a call site's arguments reach the macro at + run time, through [args], as values. + + [bound] is [rename_expr]'s idea: a local shadows a top-level name. The + binders tracked are the ones a macro body can hold — its own parameter, + [let], [loop], [fn] and [dotimes]. A [match] pattern's names and a + [restart-case] clause's parameters are not tracked, which is a gap and a + narrow one: it takes a macro body that both destructures a union and binds a + name the package also declares at the top level. *) + +let rec form_syms (f : Form.t) acc = + match f.Form.v with + | Form.Sym n -> n :: acc + | Form.List xs | Form.Vec xs | Form.Map xs -> + List.fold_left (fun a x -> form_syms x a) acc xs + | _ -> acc + +(* A leading keyword is a loop label, which [dotimes] and [while] take and + which is never a binding. *) +let peel_label = function + | ({ Form.v = Form.Kw _; _ } as k) :: rest -> ([ k ], rest) + | rest -> ([], rest) + +let rec rename_form owned alias bound (f : Form.t) : Form.t = + let keep v = { f with Form.v = v } in + let go b x = rename_form owned alias b x in + match f.Form.v with + | Form.Sym n when List.mem n owned && not (List.mem n bound) -> + keep (Form.Sym (qualify alias n)) + | Form.List (({ Form.v = Form.Sym ("let" | "loop"); _ } as hd) + :: { Form.v = Form.Vec bs; loc = bloc } :: body) -> + (* Sequential, as [let] itself is: an initialiser sees the bindings before + it and not its own. A binding position may be a destructuring pattern, + so every symbol in it binds — over-binding only ever declines to rename, + which is the safe direction. *) + let rec pairs bound acc = function + | n :: v :: rest -> pairs (form_syms n bound) (go bound v :: n :: acc) rest + | [ x ] -> (bound, go bound x :: acc) + | [] -> (bound, acc) + in + let bound, bs = pairs bound [] bs in + keep (Form.List (hd :: Form.make (Form.Vec (List.rev bs)) bloc + :: List.map (go bound) body)) + | Form.List (({ Form.v = Form.Sym "fn"; _ } as hd) + :: ({ Form.v = Form.Vec ps; _ } as pv) :: body) -> + let bound = List.fold_left (fun a p -> form_syms p a) bound ps in + keep (Form.List (hd :: pv :: List.map (go bound) body)) + | Form.List (({ Form.v = Form.Sym "dotimes"; _ } as hd) :: rest) -> + (match peel_label rest with + | lbl, ({ Form.v = Form.Vec [ n; count ]; loc = bloc } :: body) -> + keep (Form.List + (hd :: lbl + @ Form.make (Form.Vec [ n; go bound count ]) bloc + :: List.map (go (form_syms n bound)) body)) + | _ -> keep (Form.List (hd :: List.map (go bound) rest))) + | Form.List xs -> keep (Form.List (List.map (go bound) xs)) + | Form.Vec xs -> keep (Form.Vec (List.map (go bound) xs)) + | Form.Map xs -> keep (Form.Map (List.map (go bound) xs)) + | _ -> f + +(* One [defmacro] form, as an importer has to see it. The name is qualified so + that nothing a package declares becomes visible unqualified — [mac/twice] is + a call and [twice] is an unknown name, the same rule every other declaration + follows — and the body is renamed so that what the macro *answers with* + names the package's functions and macros the way the importer's file has to + spell them. + + Desugared on the way out, because [Macro.program] is handed forms that + [Parse.parse_forms] has already run [Expand.quasiquote] over and its rounds + read them with that assumed. An un-desugared one would make a quasiquoted + call look like a real one, which is the false ring BUILT.md records the + first cycle test walking into. *) +let qualify_macro owned alias (f : Form.t) : Form.t option = + match f.Form.v with + | Form.List ({ Form.v = Form.Sym "defmacro"; _ } as hd + :: ({ Form.v = Form.Sym n; _ } as nf) + :: ({ Form.v = Form.Vec ps; _ } as pv) :: body) -> + let bound = List.fold_left (fun a p -> form_syms p a) [] ps in + Some + (Expand.quasiquote + { f with + Form.v = + Form.List + (hd :: { nf with Form.v = Form.Sym (qualify alias n) } :: pv + :: List.map (rename_form owned alias bound) body) }) + (* Malformed, and not this function's business to say so: the package's own + parse runs over the same form and [Parse] has the wording. *) + | _ -> None + +(* Two packages may be reached along two routes and both arrive here, so the + set is deduped by name before it goes anywhere near [Macro] — a defmacro + twice over is a [defn] declared twice, refused by the checker for a reason + nobody would recognise. *) +let macro_union (a : Form.t list) (b : Form.t list) = + let named f = + match f.Form.v with + | Form.List (_ :: { Form.v = Form.Sym n; _ } :: _) -> Some n + | _ -> None + in + let have = List.filter_map named a in + a @ List.filter (fun f -> match named f with + | Some n -> not (List.mem n have) + | None -> true) b + +(* ── Reading an import form ───────────────────────────────────────── + Which packages a file names, read out of the forms rather than out of the + parse — and this is the whole of what had to move earlier for a package to + be allowed a macro. + + It is not a second import resolver, which is the thing the refusal this + replaces was right to be wary of. It does not recurse, it resolves no path + and it decides nothing: it reads one shape and hands the answer to + [import], which is still the only thing that walks a package graph, still + the only thing that keeps [seen] and [open_], and still the only thing that + refuses a cycle. Two resolvers can disagree. A reader and a resolver cannot. + + A malformed import is skipped rather than complained about. [Parse] runs + over the same form moments later and already has the wording for it, so + saying it here would only mean saying it twice, differently. + + Nothing is lost by reading these before expansion, because there is no + import an expansion could produce: [Parse.decl] dispatches on the head and a + macro name is not one of the heads it knows, so a macro call at the top + level is not a thing. *) +let imports_of (forms : Form.t list) = + List.filter_map + (fun (f : Form.t) -> + match f.Form.v with + | Form.List [ { Form.v = Form.Sym "import"; _ }; + { Form.v = Form.Sym a; _ }; + { Form.v = Form.Str p; _ } ] -> Some (a, p, f.Form.loc) + | _ -> None) + forms + (* ── Visibility ────────────────────────────────────────────────────── *) (* The one rule so far: [main] is not a name a package offers. @@ -633,7 +793,24 @@ let real dir = try Unix.realpath dir with Unix.Unix_error _ -> dir expander needs, since every [defmacro] has to be compiled before anything that calls it. A ring has no such order, so it is named and refused here rather than resolved arbitrarily by whichever package happened to be read - first. Odin forbids cycles for the same reason. *) + first. Odin forbids cycles for the same reason. + + That order is now being spent rather than merely promised. A package's + [defmacro] used to be refused by name, on the argument that collecting one + would need the package's own imports resolved at the Form level before this + function ran — a second import resolver, and two resolvers can disagree. + What the refusal did not notice is that the *file being compiled* is parsed + before this function runs too, so the ordering problem was never specific to + packages: no shape of this feature can leave import resolution where it was. + + So it moved, and only the reading of an import form moved with it. + [imports_of] reads the shape; this function still does every bit of the + resolving, in the order it already had. Each package's nested imports are + resolved *before* its own files are parsed, their macros are ambient in + [Parse.imported_macros] while that parse runs, and the package's own macros + come back out qualified at the end, where [owned] is complete — after + [Cimport] has generated the header's declarations, so a macro quasiquoting + [(BeginDrawing)] names [rl/BeginDrawing] like everything else. *) let rec import ~seen ~open_ ~loc alias dir = let dir' = real dir in (* Checked before [seen], because a cycle's second arrival is also a repeat @@ -654,51 +831,60 @@ let rec import ~seen ~open_ ~loc alias dir = (snd (List.nth open_ i)) (String.concat " -> " names) | None -> ()); match Hashtbl.find_opt seen dir' with - | Some previous when String.equal previous alias -> + | Some (previous, macros) when String.equal previous alias -> (* Already in, under the same name, and not still open. Importing it again - is a no-op, which is what lets two packages both depend on a third. *) - { decls = []; csrcs = []; lflags = []; pkgs = [] } - | Some previous -> + is a no-op, which is what lets two packages both depend on a third. + + A no-op for declarations only. The macros are handed back every time, + because they are not a contribution to the finished program — they are + what a *parse* needs in front of it, and the second importer's parse has + not happened yet. [macro_union] dedupes them where they land. *) + { decls = []; csrcs = []; lflags = []; pkgs = []; macros } + | Some (previous, _) -> fail loc "%s is imported as %s here and as %s elsewhere; one directory is one set \ of names, so the two cannot both be true" dir alias previous | None -> - Hashtbl.replace seen dir' alias; + Hashtbl.replace seen dir' (alias, []); let open_ = open_ @ [ (dir', alias) ] in let one_file = is_package_file dir in let files = if one_file then [ dir ] else entries dir ".flan" in if files = [] then fail loc "the package at %s has no .flan file" dir; - let ds = - List.concat_map - (fun f -> - let forms = Reader.read_file f in - (* A defmacro in a package is refused by name, and here is the only - place that can see one: by the time [Parse] is finished, a - defmacro is an ordinary [Ast.Defn] and the word is gone. + (* Read once. The forms are wanted twice — for the imports below and for + the macros at the end — and reading a file twice is the kind of second + opinion this module spends its comments warning about. *) + let sources = List.map (fun f -> (f, Reader.read_file f)) files in + (* What this package imports, resolved first and relative to itself. Its + declarations come back already qualified under their own aliases, so the + rename below leaves them alone: they are not in [owned]. - It is a real gap and not an oversight. The expander collects - macros from the prelude and from the file being compiled; to - collect them from a package it would have to resolve that - package's own imports first, at the Form level, before this - function -- which is a second import resolver. The refusal says - that rather than letting the call arrive at the checker as an - unknown name. *) - List.iter - (fun (form : Form.t) -> - match form.Form.v with - | Form.List ({ Form.v = Form.Sym "defmacro"; _ } - :: { Form.v = Form.Sym n; _ } :: _) -> - Loc.fail form.Form.loc - "%s is a macro, and macros are not imported yet. A \ - defmacro has to be compiled before the call it expands, \ - and the expander collects them from the prelude and from \ - the file being compiled -- not from a package, whose own \ - imports would have to be resolved first. Move it into \ - the file that calls it" n - | _ -> ()) - forms; - Parse.program forms) - files + Read out of the forms rather than out of the parse, because the parse is + what needs the answer: a package's own file may call a macro of a + package it imports, and that macro has to be collected before the file + naming it is parsed. *) + let nested = + List.concat_map + (fun (_, forms) -> + List.map + (fun (a, path, dloc) -> + (* Relative to the package itself: for a directory that is the + directory, for a single file the one it sits in. *) + let file = if one_file then dir else Filename.concat dir "." in + let sub = resolve_dir ~file dloc path in + import ~seen ~open_ ~loc:dloc a sub) + (imports_of forms)) + sources + in + let nested_macros = + List.fold_left (fun acc r -> macro_union acc r.macros) [] nested + in + (* The package's own files, parsed with what it imported in front of them + and nothing else. A parent's macros are deliberately not here: this + package did not import that parent, and a name it never asked for is not + one it should be able to call. *) + let ds = + Parse.with_imported nested_macros + (fun () -> List.concat_map (fun (_, forms) -> Parse.program forms) sources) in (* [main] is the importer's, always. A package that called its own would get the importer's instead — silently, since the name still resolves — @@ -709,22 +895,6 @@ let rec import ~seen ~open_ ~loc alias dir = "the package %s calls main, and main belongs to the program that \ imports it, not to a package" dir) ] ds; - (* What this package imports, resolved first and relative to itself. Its - declarations come back already qualified under their own aliases, so the - rename below leaves them alone: they are not in [owned]. *) - let nested = - List.filter_map - (fun (d : Ast.decl) -> - match d.Ast.d with - | Ast.Import (a, path) -> - (* Relative to the package itself: for a directory that is the - directory, for a single file the one it sits in. *) - let file = if one_file then dir else Filename.concat dir "." in - let sub = resolve_dir ~file d.Ast.dloc path in - Some (import ~seen ~open_ ~loc:d.Ast.dloc a sub) - | _ -> None) - ds - in (* Every header the package names, read, and turned into the same [declare-c] declarations a human would have written. Done here, before anything below looks at what the package declares, so the generated ones @@ -929,9 +1099,35 @@ let rec import ~seen ~open_ ~loc alias dir = (fun (n, why) -> (qualify alias n, qualify alias n ^ ": " ^ why)) r.Cimport.hidden) imported + (* [alias/main] is not a name, and it is recorded here rather than + recomputed later. [hidden_of] used to answer this by re-parsing the + package's files, which was affordable while a package could not hold a + macro; now that it can, that second parse would run with nothing + ambient and fail on every package whose own functions call its own + macros. The fact is known right here, so it is carried. *) + @ (if List.exists (fun d -> Ast.declared_name d = Some "main") ds then + [ (qualify alias "main", + Printf.sprintf + "%s is not a name: %s declares a main, and a main is an entry \ + point rather than something a package offers" + (qualify alias "main") dir) ] + else []) in + (* The package's macros, as an importer has to see them, built here because + here is where [owned] is both complete and still beside the forms the + author wrote. Its imports' macros travel on with them: a nested + package's names are flattened into the finished program under their own + alias, so [q/foo] is callable from the program that imported [p], and a + macro is no different. *) + let mine = + List.concat_map + (fun (_, forms) -> List.filter_map (qualify_macro owned alias) forms) + sources + in + let macros = macro_union nested_macros mine in + Hashtbl.replace seen dir' (alias, macros); let here = - { decls; csrcs; lflags; + { decls; csrcs; lflags; macros; pkgs = [ { alias; dir; owns = owned; pcsrcs = csrcs; plflags = lflags; phidden } ] } in @@ -948,53 +1144,71 @@ let rec import ~seen ~open_ ~loc alias dir = by construction and a package may be declared after the one that uses it. What will need the order is the macro expander, which cannot work that way — a [defmacro] has to be compiled before the call it expands — - and it will read [pkgs]. *) + and it is [macros] that reads it. *) List.fold_left (fun acc p -> { decls = acc.decls @ p.decls; csrcs = acc.csrcs @ p.csrcs; lflags = acc.lflags @ p.lflags; - pkgs = acc.pkgs @ p.pkgs }) - { decls = []; csrcs = []; lflags = []; pkgs = [] } + pkgs = acc.pkgs @ p.pkgs; + macros = macro_union acc.macros p.macros }) + empty (nested @ [ here ]) (* What an import did *not* bring: the names an importer might reasonably write - and that are not there, each with the reason it is not. *) -let hidden_of (t : t) = - List.concat_map (fun (p : pkg) -> p.phidden) t.pkgs - @ List.filter_map - (fun (p : pkg) -> - let ds = - List.concat_map (fun f -> Parse.program (Reader.read_file f)) - (if is_package_file p.dir then [ p.dir ] else entries p.dir ".flan") - in - if List.exists (fun d -> Ast.declared_name d = Some "main") ds then - Some (qualify p.alias "main", - Printf.sprintf - "%s is not a name: %s declares a main, and a main is an entry \ - point rather than something a package offers" - (qualify p.alias "main") p.dir) - else None) - t.pkgs + and that are not there, each with the reason it is not. Every entry is + recorded by [import] as it goes; there is no second look at the package. *) +let hidden_of (t : t) = List.concat_map (fun (p : pkg) -> p.phidden) t.pkgs (* ── The one entry point ───────────────────────────────────────────── *) -let program ~file (decls : Ast.decl list) : t = +(* Forms in rather than declarations, and that is the whole of the phase + change. The order is still [Reader] -> [Parse] -> [Load] -> [Check]; what + moved is who calls [Parse], because the file's own parse is the one that + needs a package's macros and it used to happen before this function was + reached. Nothing runs out of order: the imports are read, resolved, and only + then is the file parsed with their macros in front of it. + + [parse] is a parameter because there are two of them and the difference + matters to the daemon — [Parse.program] stops at the first bad declaration + and raises [Loc.Error], [Parse.program_all] reports every one and raises + [Loc.Errors]. See the note above them. + + [Parse.imported_macros] is extended rather than replaced, and restored on + the way out. Extended because [Session.eval] has already set the session's + own set when it calls this, and an evaluation may add an import without + losing what the file imported. Restored because a compiler process builds + more than one program and a macro left ambient is a name that works until + somebody reorders the tests. *) +let program ?(parse = Parse.program) ~file (forms : Form.t list) : t = let seen = Hashtbl.create 8 in + let imported = + List.fold_left + (fun acc (alias, path, dloc) -> + let dir = resolve_dir ~file dloc path in + let p = import ~seen ~open_:[] ~loc:dloc alias dir in + { decls = acc.decls @ p.decls; + csrcs = acc.csrcs @ p.csrcs; + lflags = acc.lflags @ p.lflags; + pkgs = acc.pkgs @ p.pkgs; + macros = macro_union acc.macros p.macros }) + empty + (imports_of forms) + in + let decls = + Parse.with_imported (macro_union !Parse.imported_macros imported.macros) + (fun () -> parse forms) + in let t = List.fold_left (fun acc (d : Ast.decl) -> match d.Ast.d with - | Ast.Import (alias, path) -> - let dir = resolve_dir ~file d.Ast.dloc path in - let p = import ~seen ~open_:[] ~loc:d.Ast.dloc alias dir in - { decls = acc.decls @ p.decls; - csrcs = acc.csrcs @ p.csrcs; - lflags = acc.lflags @ p.lflags; - pkgs = acc.pkgs @ p.pkgs } + (* Resolved above, from the form it was read out of. An [Ast.Import] + here is the same import arriving a second time and contributes + nothing; dropping it is what keeps one directory to one visit. *) + | Ast.Import _ -> acc | _ -> { acc with decls = acc.decls @ [ d ] }) - { decls = []; csrcs = []; lflags = []; pkgs = [] } - decls + imported decls in (* Said here rather than left to the checker: [sim/main] would otherwise be "unknown name", which is true and unhelpful — the name is missing on diff --git a/lib/macro.ml b/lib/macro.ml index 9be9912..97c6b9f 100644 --- a/lib/macro.ml +++ b/lib/macro.ml @@ -9,10 +9,14 @@ (* ── Which names are macros ──────────────────────────────────────── A [defmacro] is an [Ast.Defn] by the time [Parse] is finished with it, so the word only survives in the form and collecting them is a scan of the top - level. It is the prelude's macros plus the file's, and not an imported - package's: [Load] learns a package's imports by parsing it, so collecting - from one would mean a second import resolver running over Forms. A defmacro - in an imported package is refused by name instead. *) + level. It is the prelude's macros, the file's own, and an imported + package's. + + The third of those does not arrive by scanning anything here. [Load] is the + one thing that knows what a package is called from outside, and it now hands + its answer over in [Parse.imported_macros] — qualified under the alias and + desugared — so this file's only new job is to put that set in the same pot + as the other two. See the header of [Load.import]. *) let macro_name (f : Form.t) = match f.Form.v with @@ -304,6 +308,17 @@ let program (forms : Form.t list) : Form.t list = | _ -> None) forms in + (* An import's macros, and they go in beside [mine] rather than beside + [prelude]: a package macro may call another macro of its own package, so + it is exactly as much a candidate for the rounds below as one written + here. Their names carry a slash, so nothing they hold can collide with + [mine] or with the prelude's. *) + let imported = + List.filter_map + (fun f -> Option.map (fun n -> (n, f)) (macro_name f)) + !Parse.imported_macros + in + let mine = imported @ mine in let all = prelude @ List.map fst mine in (* The common case by a wide margin, and the reason a build that uses no macro pays nothing: a file that calls none costs one scan and no diff --git a/lib/parse.ml b/lib/parse.ml index ca23be8..e498b2b 100644 --- a/lib/parse.ml +++ b/lib/parse.ml @@ -995,6 +995,24 @@ and variant (f : Form.t) : Ast.variant = unknown name, which is wrong but not silent. *) let expander : (Form.t list -> Form.t list) ref = ref (fun fs -> fs) +(* The defmacros an import brought in: already qualified under the alias the + package was imported as, and already quasiquote-desugared, so they are in + exactly the shape [Macro] hands its own round-0 set. + + A ref, for the same reason [expander] is one — [Load] sits below [Macro] and + above this file, so there is no call it could make instead — and set rather + than passed because the parse that needs them is not always the parse that + resolved them: [Session.eval] parses one [defn] for C-c C-c, long after the + import that supplied the macro it calls. [Load.program] sets it around the + parse it drives and restores it afterwards; the session sets it around the + whole of an evaluation. Empty is the ordinary case and costs nothing. *) +let imported_macros : Form.t list ref = ref [] + +let with_imported (ms : Form.t list) (f : unit -> 'a) : 'a = + let saved = !imported_macros in + imported_macros := ms; + Fun.protect ~finally:(fun () -> imported_macros := saved) f + (* Two entry points and not one function with a flag, and the reason is the daemon. [Loc.Errors] is a second exception, and the handlers in the session and in the daemon name only [Loc.Error] — so a list reaching them would be diff --git a/lib/session.ml b/lib/session.ml index 1fe1103..293d5c3 100644 --- a/lib/session.ml +++ b/lib/session.ml @@ -35,6 +35,11 @@ type t = { mutable env : Check.env; (* the same, as the checker sees it *) host : Tast.program; (* what the process was built from *) pkgs : Load.pkg list; (* alias, directory, names owned *) + (* The defmacros the imports brought in, qualified. Held rather than + re-derived because C-c C-c parses one form with no import in sight, and a + macro that works on the first build and not on the reload is worse than + one that never existed. *) + mutable macros : Form.t list; mutable thunks : int; (* expression evaluations so far *) (* Whether the modules this session emits carry DWARF. It belongs to the session rather than to each call because it has to match the process the @@ -67,10 +72,10 @@ let rec same_const (a : Tast.expr) (b : Tast.expr) = | _ -> false let create ?(debug = false) ~file () = - let l = Load.program ~file (Parse.program (Reader.read_file file)) in + let l = Load.program ~file (Reader.read_file file) in let p, env = Check.program_with_env l.Load.decls in ({ file; decls = l.Load.decls; program = p; env; host = p; pkgs = l.Load.pkgs; - thunks = 0; debug }, l) + macros = l.Load.macros; thunks = 0; debug }, l) (* Which package a file being edited belongs to, if any. @@ -315,13 +320,18 @@ type change = { accepts every other change. *) let eval ?(origin = "") ?pause t src : change = let forms = Reader.read_all ~file:origin src in + Parse.with_imported t.macros @@ fun () -> (* 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 why the accumulated list is the post-Load one: re-evaluating a file that imports something would otherwise append a second copy of the import and the duplicate-name pass would reject it. *) let incoming = - let ds = (Load.program ~file:t.file (Parse.program forms)).Load.decls in + let l = Load.program ~file:t.file forms in + (* An evaluated import adds to the session's set, so a macro brought in by + C-c C-k is there for the C-c C-c after it. *) + t.macros <- l.Load.macros; + let ds = l.Load.decls in match package_of t origin with | None -> ds | Some p -> diff --git a/test/programs/pkg-macro-bare.flan b/test/programs/pkg-macro-bare.flan new file mode 100644 index 0000000..f99a51d --- /dev/null +++ b/test/programs/pkg-macro-bare.flan @@ -0,0 +1,14 @@ +;;;; The other half of the qualified-name rule: importing a package makes +;;;; nothing globally visible. [mac/twice] is the macro's name here and [twice] +;;;; is not a name at all, exactly as for a defn the package declares. +;;;; +;;;; It comes back as an unknown *function* rather than as an unknown macro, +;;;; and that is the honest answer: only [mac/twice] is in the expander's set, +;;;; so the walk never sees a head it recognises and the call reaches the +;;;; checker as what it looks like. Never built: the refusal is the test. + +(import mac "pkgs/mac") + +(defn main [] i32 + (print (twice 4)) + 0) diff --git a/test/programs/pkg-macro-ring.flan b/test/programs/pkg-macro-ring.flan new file mode 100644 index 0000000..d9439a5 --- /dev/null +++ b/test/programs/pkg-macro-ring.flan @@ -0,0 +1,7 @@ +;;;; The ring refusal, with the macros coming from a package. Never built. + +(import r "pkgs/macring") + +(defn main [] i32 + (r/ping 1) + 0) diff --git a/test/programs/pkg-macro-spin.flan b/test/programs/pkg-macro-spin.flan new file mode 100644 index 0000000..ad813c7 --- /dev/null +++ b/test/programs/pkg-macro-spin.flan @@ -0,0 +1,8 @@ +;;;; The bound on a macro that does not settle, with the macro coming from a +;;;; package. Never built. + +(import s "pkgs/macspin") + +(defn main [] i32 + (s/spin) + 0) diff --git a/test/programs/pkg-macro.flan b/test/programs/pkg-macro.flan index 5dd6cfb..c749a62 100644 --- a/test/programs/pkg-macro.flan +++ b/test/programs/pkg-macro.flan @@ -1,14 +1,35 @@ ;;;; A macro in an imported package. ;;;; -;;;; The expander collects defmacros from the prelude and from the file being -;;;; compiled. Collecting them from a package would mean resolving that -;;;; package's own imports at the Form level, before Load runs -- a second -;;;; import resolver -- so it does not, and says so. Left alone the call would -;;;; arrive at the checker as an unknown name, which is the failure shape this -;;;; codebase refuses to ship. Never built: the refusal is the test. +;;;; This used to be the refusal's test. The refusal said that collecting a +;;;; package's macros would need that package's imports resolved at the Form +;;;; level before [Load] ran -- a second import resolver. What it did not +;;;; notice is that the *file being compiled* is parsed before [Load] runs too, +;;;; so no shape of the feature could have left import resolution where it was. +;;;; [Load.program] takes forms now: it reads the import forms, resolves them +;;;; with the one resolver it always had, and parses the file with the +;;;; packages' macros in front of it. +;;;; +;;;; The rule is the one every other declaration follows. (mac/twice 4) is a +;;;; call and (twice 4) is an unknown name -- see the refusal beside this one +;;;; in test_acceptance.ml. (import mac "pkgs/mac") +;; A macro of the program's own, coexisting with the package's. +(defmacro tenfold [args] + `(* ~(at args 0) 10)) + +(defn show [n i32] () (print n) (println "")) + (defn main [] i32 - (print (mac/double 4)) + (show (mac/twice 4)) ; 8 + (show (mac/quad 3)) ; 12 + (show (mac/doubled 5)) ; 10 + (show (mac/also-twice 6)) ; 12 + (show (mac/shadowed 9)) ; 10 + (show (mac/quadruple 2)) ; 8 + (show (tenfold 7)) ; 70 + ;; The program's macro over the package's, and the package's over a prelude + ;; one: all three sets are in the same module and the walk is bottom up. + (show (tenfold (mac/twice 3))) ; 60 0) diff --git a/test/programs/pkgs/mac/mac.flan b/test/programs/pkgs/mac/mac.flan index 9bcd330..fb25972 100644 --- a/test/programs/pkgs/mac/mac.flan +++ b/test/programs/pkgs/mac/mac.flan @@ -1,7 +1,50 @@ -;;;; A package that declares a macro, which is a thing a package may not do -;;;; yet. The refusal is the test; this is never built. +;;;; A package that declares macros. +;;;; +;;;; Its names arrive at an importer qualified, exactly as a defn's do: the +;;;; program that imports this directory as [mac] writes (mac/twice 4), and +;;;; (twice 4) is an unknown name there. Nothing becomes globally visible by +;;;; importing a package, macros included. +;;;; +;;;; Inside the package the names are the package's own, unqualified, which is +;;;; the same rule every other declaration here follows. +(defn double [n i32] i32 (* n 2)) + +;; The plain case: one macro, nothing else needed to compile it. (defmacro twice [args] `(+ ~(at args 0) ~(at args 0))) -(defn double [n i32] i32 (* n 2)) +;; A macro that quasiquotes a call to another macro of this package. That is +;; *output*, not a compile-order dependency -- the call is part of what this +;; macro answers and is expanded again after it returns -- so it needs nothing +;; compiled first. What it does need is the name coming out qualified, because +;; the answer lands in the importer's file, where [twice] is not a name. +(defmacro quad [args] + `(twice (twice ~(at args 0)))) + +;; And one whose output names a *function* of this package, which has the same +;; problem and the same answer. +(defmacro doubled [args] + `(double ~(at args 0))) + +;; [wrap] takes a form-valued expression and answers one, so it is a macro +;; another macro's *body* can call for real. +(defmacro wrap [args] + `(do ~(at args 0))) + +;; A macro that really calls another, outside a quasiquote. This one *is* a +;; compile-order dependency: [wrap] has to be compiled and loaded before this +;; body will compile at all, which is what the rounds in [Macro] are for, and +;; it is the case a quasiquoted call deliberately is not. +(defmacro also-twice [args] + (wrap `(+ ~(at args 0) ~(at args 0)))) + +;; A shadowing local named like a top-level of this package. The rename must +;; leave it alone, or the expansion would name [mac/double] where the author +;; wrote a let binding. +(defmacro shadowed [args] + `(let [double ~(at args 0)] + (+ double 1))) + +;; The package's own function, calling the package's own macro unqualified. +(defn quadruple [n i32] i32 (quad n)) diff --git a/test/programs/pkgs/macring/macring.flan b/test/programs/pkgs/macring/macring.flan new file mode 100644 index 0000000..dfdd6c8 --- /dev/null +++ b/test/programs/pkgs/macring/macring.flan @@ -0,0 +1,10 @@ +;;;; A ring of macros, in a package. Neither body can be compiled first, so +;;;; there is no order to compile them in -- and an import's macros go through +;;;; the same rounds as the file's own, so the refusal has to fire here too. +;;;; Nothing in this package calls them, so the ring is found by the importer. + +(defmacro ping [args] + (pong args)) + +(defmacro pong [args] + (ping args)) diff --git a/test/programs/pkgs/macspin/macspin.flan b/test/programs/pkgs/macspin/macspin.flan new file mode 100644 index 0000000..c84c254 --- /dev/null +++ b/test/programs/pkgs/macspin/macspin.flan @@ -0,0 +1,8 @@ +;;;; A package macro that expands into a call to itself and does not get +;;;; smaller. Not a ring -- the call is inside a quasiquote, so it is output -- +;;;; and so it is bounded rather than refused, with the macro named at the call +;;;; site. The name in that message is the qualified one, because that is what +;;;; the importer wrote. + +(defmacro spin [args] + `(spin ~@args)) diff --git a/test/test_acceptance.ml b/test/test_acceptance.ml index bffe3cb..3647583 100644 --- a/test/test_acceptance.ml +++ b/test/test_acceptance.ml @@ -34,7 +34,7 @@ let compile ?(opt = "-O2") ?(checks = true) ?(dev = false) path = in (* Through [Load], so a program with an (import ...) is buildable here: it brings back the package's C shim and linker arguments as well. *) - let l = Load.program ~file:path (Parse.program (Reader.read_file path)) in + let l = Load.program ~file:path (Reader.read_file path) in let p = Check.program l.Load.decls in (* [Reach.link] decides the link from the program: a package nothing reachable calls into hands over no C and no linker argument, and its @@ -1362,7 +1362,7 @@ let () = caused it. None of these is built; being refused is the whole test. *) let refuses name path needle = let attempt () = - let l = Load.program ~file:path (Parse.program (Reader.read_file path)) in + let l = Load.program ~file:path (Reader.read_file path) in ignore (Check.program l.Load.decls) in match attempt () with @@ -1420,12 +1420,21 @@ let () = refusal that says only "there is a cycle" leaves them to find it. The ring is a -> b -> c -> a, and the message closes it by repeating the package it came back to. *) - (* A package may not declare a macro yet, and the reason is the ordering: - collecting one would mean resolving that package's own imports over - Forms, before Load runs. Refused where the defmacro is written rather - than where it is called, because that is where the fix goes. *) - refuses "a macro in an imported package" "programs/pkg-macro.flan" - "macros are not imported yet"; + (* A package may declare a macro, and its name is the package's: importing + one makes nothing globally visible, macros included. This used to be the + refusal "macros are not imported yet"; the working half is + [outputs "a macro in an imported package"] below, and this is the half + the rule needs — that the bare name is still nothing. *) + refuses "a package's macro is not visible unqualified" + "programs/pkg-macro-bare.flan" "unknown function twice"; + (* And both non-termination refusals, with the macros coming from a + package. They are different failures — a ring has no compile order, a + macro that quasiquotes itself has one and just never stops — and an + import must not quietly turn either into the other. *) + refuses "a ring of macros in a package" "programs/pkg-macro-ring.flan" + "none can be compiled first"; + refuses "a package macro that does not settle" + "programs/pkg-macro-spin.flan" "expanding s/spin did not settle"; refuses "an import ring" "programs/pkg-cycle.flan" "round a ring: a -> b -> c -> a"; refuses "two mains in one program" "programs/pkg-two-mains.flan" @@ -1499,7 +1508,7 @@ let () = else None in let wasm_build ?(opt = "-O2") path out = - let l = Load.program ~file:path (Parse.program (Reader.read_file path)) in + let l = Load.program ~file:path (Reader.read_file path) in let p = Check.program l.Load.decls in let p, csrcs, lflags = Reach.link l p in ignore @@ -2889,7 +2898,7 @@ ERR@7 unexpected token: not the kind the caller was reading ("flan-dbg-" ^ Filename.remove_extension (Filename.basename path) ^ if dev then "-dev" else "") in - let l = Load.program ~file:path (Parse.program (Reader.read_file path)) in + let l = Load.program ~file:path (Reader.read_file path) in let p = Check.program l.Load.decls in let pnames = List.filter_map diff --git a/test/test_reload.ml b/test/test_reload.ml index e686a18..acf64fa 100644 --- a/test/test_reload.ml +++ b/test/test_reload.ml @@ -33,7 +33,7 @@ let tmp name = Filename.concat scratch ("flan-reload-" ^ name) let checked path = Check.program - (Load.program ~file:path (Parse.program (Reader.read_file path))).Load.decls + (Load.program ~file:path (Reader.read_file path)).Load.decls let ms f = let t0 = Unix.gettimeofday () in diff --git a/test/test_sanitize.ml b/test/test_sanitize.ml index 3176759..deeb03a 100644 --- a/test/test_sanitize.ml +++ b/test/test_sanitize.ml @@ -70,7 +70,7 @@ let compile ~sanitize ~checks path = (if sanitize then "s" else "p") (Filename.remove_extension (Filename.basename path))) in - let l = Load.program ~file:path (Parse.program (Reader.read_file path)) in + let l = Load.program ~file:path (Reader.read_file path) in let p = Check.program l.Load.decls in let p, csrcs, lflags = Reach.link ~dev:false l p in ignore diff --git a/test/test_session.ml b/test/test_session.ml index 413bd0b..dbfe7b1 100644 --- a/test/test_session.ml +++ b/test/test_session.ml @@ -25,7 +25,7 @@ let has hay needle = reason is the part that has to survive a refactor. *) let checked_program file = Check.program - (Load.program ~file (Parse.program (Reader.read_file file))).Load.decls + (Load.program ~file (Reader.read_file file)).Load.decls let refuses ?(file = "programs/reload.flan") name src reason = let t, _ = Session.create ~file () in diff --git a/test/test_valgrind.ml b/test/test_valgrind.ml index 94620b9..499caa0 100644 --- a/test/test_valgrind.ml +++ b/test/test_valgrind.ml @@ -98,7 +98,7 @@ let compile ~checks path = (if checks then "c" else "u") (Filename.remove_extension (Filename.basename path))) in - let l = Load.program ~file:path (Parse.program (Reader.read_file path)) in + let l = Load.program ~file:path (Reader.read_file path) in let p = Check.program l.Load.decls in let p, csrcs, lflags = Reach.link ~dev:false l p in ignore diff --git a/test/test_web.ml b/test/test_web.ml index de51d96..6438e02 100644 --- a/test/test_web.ml +++ b/test/test_web.ml @@ -36,7 +36,7 @@ let have prog = Sys.command (Printf.sprintf "command -v %s > /dev/null 2>&1" pro (* One web build, through [Load] and [Reach] exactly as [flan build] does it, so a package's per-target link lines are selected here too. *) let web_build ?(opt = "-O2") path out = - let l = Load.program ~file:path (Parse.program (Reader.read_file path)) in + let l = Load.program ~file:path (Reader.read_file path) in let p = Check.program l.Load.decls in let p, csrcs, lflags = Reach.link l p in ignore @@ -281,7 +281,7 @@ let () = in let unit_main () = let path = "programs/unit-main.flan" in - let l = Load.program ~file:path (Parse.program (Reader.read_file path)) in + let l = Load.program ~file:path (Reader.read_file path) in Check.program l.Load.decls in refused "--dev --target=web" "--dev is native only" (fun () ->