A runaway instantiation refuses instead of hanging the editor

SPIKE-GENERICS.md

@@ -179,10 +179,17 @@ Read off it:
    extra body**, roughly linear. A redefinition module with one function is ~19 ms; the same redefinition of a
    generic used at 8 element types is ~45 ms.
 
-So: **redefining a generic function used at N element types costs the dev loop about `19 + 1.7 × (bodies − 1)` ms
-against a 19 ms baseline.** For the realistic case — a generic used at two or three types — that is 21–28 ms, a
-10–50% increase on the slowest part of the loop. For a `map` used at eight types it is 2.4×. It stays under the
-"redefine a whole file" cost, and it is proportional to what changed rather than to the program.
+**What the 19 ms is and is not.** It is `Build.shared` alone: `llc` plus `ld -shared`. The ~35 ms the brief
+quotes for `C-c C-c` is the whole round trip, and `dev.ml:446` pays several things around this call that are not
+in it — `Session.eval`'s read/parse/Load/check (the 1.8 ms column), writing the `.ll` and the `.o`, `deliver` and
+the `dlopen` at a frame boundary, and the wire exchange with the editor. So the two numbers are not in conflict;
+they are measuring different brackets, and the ~16 ms between them is the part this spike does not change.
+
+**The marginal number is the one that transfers, and it is invariant to which baseline it is added to: +1.7 ms
+per extra body.** In the terms the brief asks for: a generic used at two element types adds about 3.5 ms to a
+~35 ms loop (one generic that calls another, so four bodies rather than two — 10%); at three types, ~7 ms; at
+eight types, ~26 ms, which is a loop of ~61 ms rather than ~35. It stays well under the "redefine a whole file"
+cost and is proportional to what changed rather than to the size of the program.
 
 **And the half that is not implemented.** The last line of the sweep is the finding:
 
@@ -208,11 +215,11 @@ Against what a generic function has to survive, in DISCUSS.md item 15's buckets:
 
 | | |
 |---|---|
-| **Done in the spike** | one or more type variables in a signature; binding through `[t]`, `(Ptr t)`, `(Option t)`, `(Fn [t] t)` and nesting; return types mentioning a variable; left-to-right binding with substitution into later parameters so an `fn` literal gets its types; the Odin-keyed instantiation cache; generic calling generic, transitively; `(vec-new t)`; the abstract refusal pass for `+`, `-`, `*`, `/`, `%`, `=`, `!=`, `<`, `<=`, `>`, `>=`, the bitwise operators and the shifts; instantiations as ordinary `Tast.fn`s with dev cells and `Reach` edges for free |
+| **Done in the spike** | one or more type variables in a signature; binding through `[t]`, `(Ptr t)`, `(Option t)`, `(Fn [t] t)` and nesting; return types mentioning a variable; left-to-right binding with substitution into later parameters so an `fn` literal gets its types; the Odin-keyed instantiation cache; generic calling generic, transitively; `(vec-new t)`; the abstract refusal pass for `+`, `-`, `*`, `/`, `%`, `=`, `!=`, `<`, `<=`, `>`, `>=`, the bitwise operators and the shifts; instantiations as ordinary `Tast.fn`s with dev cells and `Reach` edges for free; a depth cap so runaway instantiation refuses instead of hanging |
 | **Mechanical** | the remaining builtins that take a *type name* as an argument — `pool-new`, `map-new`, `zeroed`, `uninit`, the casts — each of which reaches `type_named`/`resolve_name` by its own path, exactly as `vec-new` did (one line there fixed `vec-new`; the others are one line each); `hash` and the map key-pair path, which must refuse a `Var` rather than assume; the `fns` expansion in `session.ml` described above |
 | **Bulky, not hard** | error messages that say *where* an instantiation came from — Odin's "in instantiation of" note. Today a refusal inside an instantiated body points at the generic's source with no indication which call site asked for that type, and with three or four instantiations that is the difference between a readable refusal and a puzzle. It is a context stack in `ctx` and a `Loc.note` per frame, and it touches every `fail` under an instantiation |
-| **Fiddly** | move-only and ownership. `Types.is_move_only (Var _)` is false, but the same variable at `(Vec i32)` is move-only — so the abstract pass **cannot decide ownership at all**, and the dead-set analysis is only sound per instantiation. Today that means a generic body that moves its parameter type-checks abstractly and is caught, if at all, at one instantiation and not another. The rule has to be stated: either ownership is checked only per copy (and the abstract pass skips it, so a generic may be accepted and its instantiation refused), or type variables carry a move-only constraint, which is a constraint system and plan.org says not yet |
-| **No plan** | (1) **Unbounded instantiation.** `(defn grow [x $t] () (grow [x x]))` hangs the checker: each copy asks for a copy at `[2 t]`, forever. There is no depth cap, no size cap, and no cycle detection — and **Odin has none either**, so there is no implementation to copy. It needs a designed limit with a refusal that names the chain. (2) **Generic structs and containers.** `Types.Named` is a bare string with no parameters, so `(defstruct Pair [a $t b $t])` cannot be spelled at all — a parameterised named type is a change to `Types.t` and therefore to every backend, `Render`, DWARF and the layout calculator. (3) **`println` over a type variable.** plan.org's one compiler-provided exception; the abstract pass rejects it (`no printer for t`), see question 6. (4) **Generics across packages.** `Load` flattens imports into one namespace before checking, so it happens to work here, but a package boundary that is ever a real compilation-unit boundary would need the generic's *body* to cross it — the thing separate compilation cannot do and the reason C++ puts templates in headers |
+| **Fiddly** | move-only and ownership. `Types.is_move_only (Var _)` is false, but the same variable at `(Vec i32)` is move-only — so the abstract pass **cannot decide ownership at all**, and the dead-set analysis is only sound per instantiation. Today that means a generic body that moves its parameter type-checks abstractly and is caught, if at all, at one instantiation and not another. The rule has to be stated: either ownership is checked only per copy (and the abstract pass skips it, so a generic may be accepted and its instantiation refused), or type variables carry a move-only constraint, which is a constraint system and plan.org says not yet. One smaller thing in the same bucket, found and left alone: the abstract pass over a generic body that calls *another* generic at a concrete type generates that copy and keeps it, so plain `flan emit` can carry a body no call site asked for. It is a valid instantiation and `Reach.link` drops it, so `flan build` and `flan run` are unaffected — but the abstract pass is meant to leave nothing behind and this is the one thing it does |
+| **No plan** | (1) **Unbounded instantiation.** `(defn grow [x $t] () (grow [x x]))` asks for a copy at `[2 t]`, which asks for one at `[2 [2 t]]`, forever. Before the cap it did not fail, it *hung* — and since `Session.eval` runs this same code, the thing that hangs is `C-c C-c`, with the dev daemon wedged behind it and no error to show. That is the project's stated priority hanging on three lines of ordinary-looking Flan, so the spike stops it: a depth counter in `env`, refusing past 32 and naming the type it had reached (`spike/generics/runaway.flan`). **The number is arbitrary and the designed refusal — one that names the chain of instantiations rather than the depth it gave up at — is still open.** **Odin has no cap of its own**, so there is no implementation to copy. (2) **Generic structs and containers.** `Types.Named` is a bare string with no parameters, so `(defstruct Pair [a $t b $t])` cannot be spelled at all — a parameterised named type is a change to `Types.t` and therefore to every backend, `Render`, DWARF and the layout calculator. (3) **`println` over a type variable.** plan.org's one compiler-provided exception; the abstract pass rejects it (`no printer for t`), see question 6. (4) **Generics across packages.** `Load` flattens imports into one namespace before checking, so it happens to work here, but a package boundary that is ever a real compilation-unit boundary would need the generic's *body* to cross it — the thing separate compilation cannot do and the reason C++ puts templates in headers |
 
 The "no plan" row's first entry is the one to take seriously: it is not a missing feature, it is a hang, and it is
 reachable from three lines of ordinary-looking Flan.
@@ -311,6 +318,10 @@ to drop.
 
 ## Reproducing
 
+`dune test --root .` is green either side of this work. One caveat for anyone reproducing under load:
+`test_dev`'s `the daemon never listened` check is timing-sensitive and fails identically at `97cb77d` with
+`lib/check.ml` restored from that commit — it is not this lane's. On an idle machine it passes.
+
 ```
 dune exec --root . bin/main.exe -- run spike/generics/id.flan
 dune exec --root . bin/main.exe -- run spike/generics/swap.flan
@@ -318,5 +329,6 @@ dune exec --root . bin/main.exe -- run spike/generics/sort.flan
 dune exec --root . bin/main.exe -- run spike/generics/two-vars.flan
 dune exec --root . bin/main.exe -- run spike/generics/prelude-shapes.flan
 dune exec --root . bin/main.exe -- check spike/generics/reject.flan     # the refusal
+dune exec --root . bin/main.exe -- check spike/generics/runaway.flan    # the depth cap
 bash spike/generics/run.sh                                              # the sweep
 ```

lib/check.ml

@@ -102,6 +102,14 @@ type env = {
      [resolve_name] consults it before anything else, so the body resolves
      [t] to [i32] and every node under it is concrete. *)
   mutable subst : (string * Types.t) list;
+  (* How many instantiations deep the checker is. A generic that calls itself
+     at a *larger* type — [(defn grow [x $t] () (grow [x x]))] — asks for a
+     copy at [[2 t]], which asks for one at [[2 [2 t]]], forever. Without this
+     the checker does not fail, it hangs, and since [Session.eval] runs the
+     same code that is the editor hanging with the daemon wedged behind it.
+     Odin has no cap of its own to copy; the number is arbitrary and the
+     refusal that names the chain is still to design. *)
+  mutable depth : int;
 }
 
 let new_env () = {
@@ -122,6 +130,7 @@ let new_env () = {
   instances = [];
   tyvars = [];
   subst = [];
+  depth = 0;
 }
 
 (* Where a named type was declared, and what it has, as a note.
@@ -4554,6 +4563,13 @@ and instantiate env loc gname vars subst cparams cret =
     (* The entry goes in *before* the body is checked, which is what makes a
        recursive generic function terminate: the call to itself at the same
        types finds this and does not generate a second copy. *)
+    if env.depth >= 32 then
+      fail loc
+        "%s instantiates itself without end — the copy at (%s) asks for \
+         another at a larger type, 32 deep and still growing. A generic \
+         function may call itself, but not at a type built out of its own \
+         type variable" gname
+        (String.concat " " (List.map Types.to_string cparams));
     cache := (cparams, cret, sym) :: !cache;
     Hashtbl.replace env.fns sym (cparams, cret);
     let fn = Hashtbl.find env.generics gname in
@@ -4563,11 +4579,22 @@ and instantiate env loc gname vars subst cparams cret =
        concrete as one written out by hand. *)
     env.subst <- List.map (fun v -> (v, List.assoc v subst)) vars;
     env.tyvars <- [];
-    let restore () = env.subst <- saved_subst; env.tyvars <- saved_vars in
+    env.depth <- env.depth + 1;
+    let restore () =
+      env.subst <- saved_subst; env.tyvars <- saved_vars;
+      env.depth <- env.depth - 1
+    in
     let tfn =
       match !check_fn_ref env { fn with Ast.name = sym } with
       | tfn -> restore (); tfn
-      | exception e -> restore (); raise e
+      | exception e ->
+        restore ();
+        (* A copy whose body did not check is not a copy. Both entries go back
+           out, so a second call at the same types is the same refusal again
+           rather than a cache hit on a function that does not exist. *)
+        cache := List.filter (fun (_, _, s) -> s <> sym) !cache;
+        Hashtbl.remove env.fns sym;
+        raise e
     in
     env.instances <- tfn :: env.instances;
     sym

spike/generics/runaway.flan

@@ -0,0 +1,3 @@
+(defn grow [x $t] ()
+  (grow [x x]))
+(defn main [] () (grow 1))