Four ways C-c C-c could lie, found by trying a defconst
Asked whether a defconst could be redefined, probed it, and got ":status ok" for a change that did nothing at all - the module was built, delivered, installed, and the program went on using the old value. That is the silent-wrongness class the house rule exists to prevent, so it is now four refusals and a fix. A defconst's value is folded into its call sites - into an array length at worst, which is decided before any type resolves - so it lives in the running program's code and not only in its storage. Refused. A defenum member is the same thing: :space is erased to an i32 literal in the caller. Refused, and compared over declarations rather than over Tast.program, which carries no enums at all for exactly that reason. A defvar's initial value is deliberately not refused. 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 value comparison is structural and conservative - anything it does not recognise counts as changed. Comparing emitted text would be wrong, since Emit.const on a string allocates a name off a per-module counter and two different strings in two throwaway modules both come out as @".str.0". Third: a new global's declared initial value was being dropped. flan_dev_global callocs, so (defvar n i64 42) added at run time was silently zero. It now takes the initial value as a blob, copies it on the allocation and ignores it afterwards - the second half being where "a reload must not reset the program's state" lives. In the allocation path rather than a branch at the call site, so it cannot be got wrong at one of them. Fourth: a change with no body to publish and no storage to allocate now answers "nothing to install" instead of shipping an empty module. That is what the defconst probe actually did, and it cost the program a frame's worth of reload it did not need.
This commit is contained in:
parent
56395edd59
commit
52d3898116
23
NEXT.md
23
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 `<eval>`, so
|
||||
without it every error an editor shows points into a file that does not exist.
|
||||
|
||||
|
||||
@ -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)))
|
||||
|
||||
@ -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
|
||||
|
||||
16
lib/emit.ml
16
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
|
||||
|
||||
@ -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 = "<eval>") t src : change =
|
||||
@ -182,6 +245,7 @@ let eval ?(origin = "<eval>") 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 = "<eval>") 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 }
|
||||
|
||||
@ -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);
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user