diff --git a/NEXT.md b/NEXT.md index 414c542..f859017 100644 --- a/NEXT.md +++ b/NEXT.md @@ -434,7 +434,12 @@ void *flan_dev_global(const char *name, uint64_t); /* a new global's storage * ``` Both are idempotent, so the second module to mention a name gets what the first -one got — which is the entire point. The compiler picks per name: a name the +one got — which is the entire point. A new global's **declared initial value +travels with it**, as a constant the runtime copies on the allocation and +ignores on every call after: `calloc` alone is only right for ZII, and the +"ignores afterwards" half is where "a reload must not reset the program's +state" lives. Putting it in the allocation path rather than in a branch at the +call site means the rule cannot be got wrong at one of them. The compiler picks per name: a name the host has is a symbol (one load at a call site), a name it lacks is a registry lookup cached at install time in a module-local slot (two loads). So the common case pays nothing for the general one. @@ -551,6 +556,16 @@ Two things the session knows that no single evaluation could: | a function's signature | a cell is a bare `ptr`; every call site compiled before the change still passes the old arguments through it | | a global's type | the storage exists and has a shape — reuse reads at the wrong offsets, replacement discards the state the reload exists to preserve | | a struct's fields | the values the process is holding have the old layout | + | a `defconst`'s value | it is folded into every call site — into an array length, at worst, which is decided before any type resolves | + | a `defenum` member | `:space` is erased to an `i32` literal in the caller, so it is folded there too | + + A `defvar`'s *initial value* is deliberately **not** in that table. Its + storage holds live state the program moved past long ago, and refusing to + change the initialiser would be refusing "edit the code, keep the sand". Same + `Tast.global` record as a `defconst`, opposite answers, told apart by + `gconst`. The enum comparison runs over declarations rather than the checked + program, because `Tast.program` carries no enums at all — they are erased to + `i32` in the checker, which is the same fact that makes them unreloadable. Note what the checker catches on its own: change `helper`'s parameter type and the *caller* fails to type check first, loudly. The session's rules only @@ -602,6 +617,12 @@ can sit on the same `Session` later; it should not have gated the editor. (:op "close") ``` +An evaluation that declares nothing to install — a declaration the program +already has, with no body and no new storage — is accepted and answered with +`:note "nothing to install"` rather than by shipping an empty module. Building +one anyway reports success for a change that cannot have taken effect, and +costs the program a reload it did not need. + `:file` is not decoration: `Session.eval`'s origin defaults to ``, so without it every error an editor shows points into a file that does not exist. diff --git a/emacs/flan-dev.el b/emacs/flan-dev.el index bd21c9b..4d472eb 100644 --- a/emacs/flan-dev.el +++ b/emacs/flan-dev.el @@ -149,12 +149,17 @@ With no argument, look for `flan-dev-socket-name' up from this buffer." "Report REPLY, describing WHAT was sent." (if (equal (plist-get reply :status) "ok") (let ((fns (plist-get reply :fns)) - (names (plist-get reply :names))) + (names (plist-get reply :names)) + (note (plist-get reply :note))) (when flan-dev-echo-result - (message "%s installed in %.0fms" - (if fns (string-join fns ", ") - (if names (string-join names ", ") what)) - (or (plist-get reply :ms) 0)))) + (if note + ;; The daemon accepted it and had nothing to send. Say so rather + ;; than claiming an install that did not happen. + (message "%s: %s" (if names (string-join names ", ") what) note) + (message "%s installed in %.0fms" + (if fns (string-join fns ", ") + (if names (string-join names ", ") what)) + (or (plist-get reply :ms) 0))))) ;; The daemon reports where, so put point there when it is this buffer. (let ((loc (plist-get reply :loc)) (msg (plist-get reply :message))) diff --git a/lib/dev.ml b/lib/dev.ml index 7ec30dd..dcda915 100644 --- a/lib/dev.ml +++ b/lib/dev.ml @@ -76,6 +76,14 @@ let eval t ~code ~origin = if not (alive t) then error "the program exited; restart flan dev" else match Session.eval ~origin t.session code with + | c when not c.Session.installs -> + (* Accepted into the session and nothing to send: a declaration the + program already has, with no body and no new storage. Saying "ok" and + shipping an empty module would report success for a change that cannot + have taken effect. *) + ok + [ ":names " ^ Wire.strings c.Session.names; ":fns ()"; + ":note " ^ Wire.quote "nothing to install" ] | c -> t.n <- t.n + 1; let out = Filename.concat t.dir (Printf.sprintf "m%d.so" t.n) in diff --git a/lib/emit.ml b/lib/emit.ml index 4685d22..d57bc45 100644 --- a/lib/emit.ml +++ b/lib/emit.ml @@ -1031,7 +1031,8 @@ let redefinition ?(checks = true) ?(dev = false) ?(known = fun _ -> true) p.Tast.fns; if new_fns <> [] || new_globals <> [] then Buffer.add_string m.out - "\ndeclare ptr @flan_dev_cell(ptr)\ndeclare ptr @flan_dev_global(ptr, i64)\n"; + "\ndeclare ptr @flan_dev_cell(ptr)\n\ + declare ptr @flan_dev_global(ptr, i64, ptr)\n"; Buffer.add_char m.out '\n' end else @@ -1069,11 +1070,20 @@ let redefinition ?(checks = true) ?(dev = false) ?(known = fun _ -> true) (* sizeof, spelled the way LLVM spells it: the offset of element one of a null pointer. Cheaper than a layout calculator in OCaml that would have to agree with LLVM's on every target. *) + (* Its declared initial value travels with it, as a constant the + runtime copies on the allocation and ignores afterwards. Without + this a new (defvar n i64 42) or a new defconst would silently be + zero — calloc is only the right answer for ZII. *) + let init = Printf.sprintf "@\".init.%d\"" m.nstr in + m.nstr <- m.nstr + 1; + Buffer.add_string m.strs + (Printf.sprintf "%s = private constant %s %s\n" init + (ll g.Tast.gty) (const m g.Tast.ginit)); Buffer.add_string b (Printf.sprintf - " %s = call ptr @flan_dev_global(ptr %s, i64 ptrtoint (ptr getelementptr (%s, ptr null, i32 1) to i64))\n \ + " %s = call ptr @flan_dev_global(ptr %s, i64 ptrtoint (ptr getelementptr (%s, ptr null, i32 1) to i64), ptr %s)\n \ store ptr %s, ptr %s\n" - t (cstring m ("flan." ^ g.Tast.gname)) (ll g.Tast.gty) t + t (cstring m ("flan." ^ g.Tast.gname)) (ll g.Tast.gty) init t (globalptr g.Tast.gname))) new_globals; List.iter diff --git a/lib/session.ml b/lib/session.ml index 0544f40..5a344a8 100644 --- a/lib/session.ml +++ b/lib/session.ml @@ -37,6 +37,25 @@ type t = { let fail = Loc.fail +(* Structural, and conservative: anything this does not recognise counts as + changed. Comparing emitted text instead would be wrong — [Emit.const] on a + string allocates a name off a per-module counter, so two different strings + in two throwaway modules both come out as [@".str.0"] and compare equal. *) +let rec same_const (a : Tast.expr) (b : Tast.expr) = + match (a.Tast.e, b.Tast.e) with + | Tast.Int (x, k), Tast.Int (y, l) -> Int64.equal x y && k = l + | Tast.Float (x, k), Tast.Float (y, l) -> Float.equal x y && k = l + | Tast.Bool x, Tast.Bool y -> x = y + | Tast.Str x, Tast.Str y -> String.equal x y + | Tast.Unit, Tast.Unit -> true + | Tast.Zero x, Tast.Zero y -> Types.equal x y + | Tast.Arr xs, Tast.Arr ys -> + List.length xs = List.length ys && List.for_all2 same_const xs ys + | Tast.Make (x, xs), Tast.Make (y, ys) -> + String.equal x y && List.length xs = List.length ys + && List.for_all2 same_const xs ys + | _ -> false + let create ~file = let l = Load.program ~file (Parse.program (Reader.read_file file)) in let p = Check.program l.Load.decls in @@ -89,6 +108,21 @@ let compatible ~loc (old_ : Tast.program) (new_ : Tast.program) = (fun (h : Tast.global) -> String.equal h.Tast.gname g.Tast.gname) old_.Tast.globals with + (* A [defconst] is folded into its call sites — into an array length, at + worst, which is decided before any type resolves — so its value is in + the running program's code and not only in its storage. A [defvar]'s + initial value is the opposite case and must *not* be refused: the + storage holds live state the program has long since moved past, which + is the whole of "edit the code, keep the sand". Same record, opposite + answers, told apart by [gconst]. *) + | Some h + when h.Tast.gconst && g.Tast.gconst + && Types.equal g.Tast.gty h.Tast.gty + && not (same_const g.Tast.ginit h.Tast.ginit) -> + fail loc + "%s changes value; the running program folded the old one into its \ + code, where a reload cannot reach it. Restart to change it." + g.Tast.gname | Some h when not (Types.equal g.Tast.gty h.Tast.gty) -> (* The storage exists and has a shape. Reusing it for another one reads fields at the wrong offsets; allocating fresh storage would @@ -127,6 +161,30 @@ let compatible ~loc (old_ : Tast.program) (new_ : Tast.program) = | None -> ()) new_.Tast.structs +(* An enum member is erased to an [i32] literal in the caller — [:space] at a + call site resolves to a number and is folded there — so changing one cannot + reach code that is already compiled, exactly like a [defconst]. It has to be + compared over declarations rather than over [Tast.program], which carries no + enums at all for that same reason. *) +let compatible_enums ~loc old_ new_ = + let members (ds : Ast.decl list) = + List.filter_map + (fun (d : Ast.decl) -> + match d.Ast.d with Ast.Defenum (n, ms) -> Some (n, ms) | _ -> None) + ds + in + let before = members old_ in + List.iter + (fun (n, ms) -> + match List.assoc_opt n before with + | Some old_ms when old_ms <> ms -> + fail loc + "%s changes its members; the running program folded the old values \ + into every call site that names one. Restart to change it." + n + | _ -> ()) + (members new_) + (* ── Accepting a change ────────────────────────────────────────────── *) (* The redefinition unit is a list of top-level forms, so this is one path for @@ -135,6 +193,11 @@ type change = { ir : string; (* the module to build and send *) names : string list; (* everything the forms declared *) fns : string list; (* the subset that has a body to install *) + (* False when the module would define nothing: no body to publish and no + storage to allocate. Building and delivering one anyway reports success + for a change that cannot have had an effect, and costs the program a + frame's worth of reload it did not need. *) + installs : bool; } let eval ?(origin = "") t src : change = @@ -182,6 +245,7 @@ let eval ?(origin = "") t src : change = leaves it exactly as it was. *) let program = Check.program decls in compatible ~loc t.program program; + compatible_enums ~loc t.decls decls; let fns = List.filter (fun n -> @@ -191,6 +255,11 @@ let eval ?(origin = "") t src : change = names in let ir = Emit.redefinition ~dev:true ~known:(known t) program ~fns in + let allocates = + List.exists + (fun (g : Tast.global) -> not (known t g.Tast.gname)) + program.Tast.globals + in t.decls <- decls; t.program <- program; - { ir; names; fns } + { ir; names; fns; installs = fns <> [] || allocates } diff --git a/runtime/flan_dev.c b/runtime/flan_dev.c index 2d09b59..91eed03 100644 --- a/runtime/flan_dev.c +++ b/runtime/flan_dev.c @@ -10,8 +10,8 @@ * So a name that is new at run time is keyed by string instead. This file is * the two lookups that make that work, and deliberately nothing else: * - * flan_dev_cell(name) the cell a new function lives in - * flan_dev_global(name, size) the storage a new global lives in + * flan_dev_cell(name) the cell a new function lives in + * flan_dev_global(name, size, init) the storage a new global lives in * * Both are idempotent: the second module to mention a name gets what the first * one got. That is the whole point. Two modules that each define their own @@ -75,19 +75,27 @@ void **flan_dev_cell(const char *name) { return &e->cell; } -/* Zeroed storage for a run-time-introduced global, allocated once. +/* Storage for a run-time-introduced global, allocated once. + * + * [init] is its declared initial value, or NULL for all-zero. It is copied on + * the allocation and ignored on every call after it, which is where "a reload + * must not reset the program's state" lives: the second module to mention this + * name is a redefinition, and re-running an initialiser would throw away + * exactly what the reload exists to preserve. Doing it here rather than by a + * branch in the caller means the rule cannot be got wrong at one call site. * * A size mismatch is the layout-drift failure, caught at its first chance: the * running process has already laid this memory out, and handing back the old * allocation for a differently shaped type means the new body reads fields at * the wrong offsets and nothing ever says so. Retyping a var needs a restart. */ -void *flan_dev_global(const char *name, uint64_t size) { +void *flan_dev_global(const char *name, uint64_t size, const void *init) { entry *e = find(name); if (e == NULL) { e = intern(name); e->cell = calloc(1, size ? (size_t)size : 1); if (e->cell == NULL) die("out of memory", name); e->size = (size_t)size; + if (init != NULL && size > 0) memcpy(e->cell, init, (size_t)size); return e->cell; } if (e->size != (size_t)size) die("size changed; restart to retype", name); diff --git a/test/programs/reload.flan b/test/programs/reload.flan index 234d256..380cea8 100644 --- a/test/programs/reload.flan +++ b/test/programs/reload.flan @@ -18,6 +18,13 @@ ;;; question. (defvar spare i64) +;;; Also unused, and for the same reason: a defconst's value and an enum +;;; member are folded into every call site, so a session has to refuse changing +;;; either. Nothing here reads them, so the checker has no opinion and the +;;; session's rule is the only thing that can speak. +(defconst folded i64 7) +(defenum Colour [red 0 green 1]) + (defn helper [x i64] i64 (* x 2)) (defn bump [] i64 diff --git a/test/test_session.ml b/test/test_session.ml index bdaa9fa..fd30a2e 100644 --- a/test/test_session.ml +++ b/test/test_session.ml @@ -49,6 +49,18 @@ let () = "(defvar spare i32)" "changes type"; (* Values of the type are already in the running program's memory. *) + (* A defconst is folded into its call sites — into an array length, at worst, + which is decided before any type resolves — so its value lives in the + program's code and not only in its storage. *) + refuses "a changed defconst" + "(defconst folded i64 8)" + "changes value"; + (* An enum member is erased to an i32 literal in the caller, so the same + applies. It is compared over declarations because Tast.program carries no + enums at all, for exactly that reason. *) + refuses "a changed enum member" + "(defenum Colour [red 0 green 2])" + "changes its members"; refuses ~file:"programs/values.flan" "a restructured struct" "(defstruct P [x i32 y i32])" "changes layout"; @@ -56,6 +68,7 @@ let () = (* An ordinary redefinition, and what the session works out about it. *) let t, _ = Session.create ~file:"programs/reload.flan" in let c = Session.eval t "(defn bump [] i64 (set counter (+ counter 5)) counter)" in + if not c.Session.installs then fail "a redefined function had nothing to install"; if c.Session.fns <> [ "bump" ] then fail "redefining bump reported %s" (String.concat " " c.Session.fns); (* The prelude is in the checked program and in no accumulated AST, so a @@ -75,6 +88,21 @@ let () = | c -> if c.Session.fns <> [ "bump" ] then fail "the session did not recover" | exception Loc.Error (_, m) -> fail "the session was poisoned by a typo: %s" m); + (* A declaration the program already has, with no body and no new storage, + is accepted and has nothing to send. Building a module for it would report + success for a change that cannot have taken effect, and would cost the + program a reload it did not need. *) + (match Session.eval t "(defvar counter i64)" with + | c -> if c.Session.installs then fail "an empty change claimed to install" + | exception Loc.Error (_, m) -> fail "redeclaring a var unchanged: %s" m); + + (* A new global carries its declared initial value, copied once when the + storage is allocated and never again — calloc alone would make it zero. *) + let c = Session.eval t "(defvar started i64 42) (defn read-started [] i64 started)" in + if not (has c.Session.ir "@\".init.") then + fail "a new global's initialiser was dropped"; + if not c.Session.installs then fail "adding a global had nothing to install"; + (* Names the process was never built with go through the registry instead of binding to a symbol, and adding one is allowed where retyping one is not. *) let c = Session.eval t "(defvar fresh i64) (defn use-fresh [] i64 (set fresh 3) fresh)" in