diff --git a/docs/BUILT.md b/docs/BUILT.md index bb08a35a..a4cd75c7 100644 --- a/docs/BUILT.md +++ b/docs/BUILT.md @@ -3935,6 +3935,14 @@ appear, and no fourth: **No ordinary `defn` gained a parameter, in any program.** `flan_rt.c`'s hash and equality typedefs are untouched; `main`, the macro thunk, the startup call and the reload thunk emit the calls they always emitted. +**`handler-bind` is not free, and rounding it to zero would be wrong.** A program that establishes a handler and +captures nothing still pays: `%handler` went from 24 bytes to 32, every push writes an unconditional null into the +new field, every clause gains `ptr %env` plus an alloca and a store at entry, and `flan_signal` passes one more +argument per dispatch. Measured on `loops.flan`: +20 changed lines of x86. It is small and it is real, and every +program with conditions in it pays it — the alternative was a second clause convention beside the capturing one, +which `flan_signal` could not choose between because it calls through one C function-pointer type and cannot know +which clause matched. + ### The environment is the last argument, on exactly the bodies an `Fn` can reach The environment is the last parameter, after the transfer channel, and it is declared by **exactly the bodies a @@ -3974,6 +3982,28 @@ A redefinition module carries its own copy of every thunk, hidden. A module that the host has no cell for it to be reached through — the same shape of bug as the `Fnval` cell below, found the same way and closed before it shipped. `flan reload` with a body that widens a name builds on both backends. +**The memo is keyed on the types and the symbol is a counter**, which review found the hard way. Keyed on a *name* +derived from `mangle_ty` it was not a memo but a collision: that function flattens a whole signature into one +hyphen-joined string, so `(CFn [(Ptr i32)] i32)` and `(CFn [ptr i32] i32)` — the second over a struct someone +called `ptr` — flatten alike, and the second widening silently reused the first's thunk at the wrong arity. Both +backends compiled it without a word and neither ran it. `fn-thunk-share.flan` is that exact pair, and it prints 5 +and 17. (`mangle_ty`'s ambiguity is older than this lane and is still there for the generic instantiation names it +was written for; at one type's granularity it is hard to reach, at a whole signature's it is a line of Flan away.) + +**A generic that binds its variable *through* a function type needed both halves.** `(defn apply2 [f (Fn [$t] $t) +x $t] …)` called as `(apply2 bump 1)` is the shape a bare name has to reach now that a `defn`'s address carries +`CFn`, and it broke in two places that fail apart: `bind_ty` had no arm admitting a `CFn` argument at an `Fn` +pattern (so the instantiation was refused outright — a regression against a program that compiled before this +lane), and `generic_call`'s catch-up pass did not widen it (so the call site handed one word to an instance +declaring two). A parameter that still mentions a variable is checked with *no expectation*, by design — there is +nothing to expect until the argument has spoken — so `expect`, where the widening otherwise lives, never sees the +pair. The same gap hid the new half: a `CFn` argument at a `(CFn [$t] $t)` parameter had no arm either and fell +through to plain equality. `fn-generic.flan` covers both. + +The corpus missed all of it, and the reason is worth keeping: the prelude's higher-order functions bind `$t` from +an *earlier* argument, so `subst_ty` has already made the parameter concrete by the time `bind_ty` sees it. +`(map-in-place s double)` and `higher-order.flan` really were still working. + The reverse coercion does not exist — there is nowhere for an environment to go — and is refused by the ordinary type message, which names both spellings (`fn-cfn-narrow.flan`). A capturing literal written into a `CFn` position is refused by name, with what it captured and the fix in the sentence (`fn-cfn-captures.flan`). @@ -4026,6 +4056,14 @@ The two types made this pass narrower rather than wider, which is the point of h `CFn` has already promised what the analysis would otherwise have to prove, and nothing written against one is ever examined. +**The clean set is the enumeration, not the suspect set**, and that is a correction. It read the other way round — +`Field`, `CaseField` and `Deref` named as suspect, everything else clean — and had a hole exactly where a list like +this cannot: `(at s 0)` over a slice of `Fn` is a `Prim`, so it came out clean while the `Vec`, struct and pointer +spellings of the same act were refused. Nothing can write an `Fn` into a slice today, so it was unreachable; but +the pass claims its enumeration is closed, and a default of "clean" is how that claim stops being true without +anyone noticing. `fn-escape-at.flan` pins it. The same inversion fixed which of the two refusal messages an index +read gets. + Two of those arms are there because leaving them out is unsound rather than merely conservative, and each has a program. **A function value read out of an environment** (`fn-escape-copy.flan`): a lifted body holds *copies* of what it captured, read back with `Field(Deref env, i)`, so a copy of a captured function value carries whatever diff --git a/lib/check.ml b/lib/check.ml index 83feaf4c..43c244f2 100644 --- a/lib/check.ml +++ b/lib/check.ml @@ -657,12 +657,17 @@ let rec capture ctx loc name = match ctx.outer_what, outer with | Some what, Some (outer : binding) -> if outer.bty = Types.Dyn then + (* [what] is a descriptor — "an fn", "a handler" — so it reads as the + subject of a sentence and nowhere else. It used to be substituted + into a noun slot as well, which produced "the environment an fn is + handed"; the environment belongs to *this* capture and naming it + twice said less, not more. *) Loc.failk "check/capture-dyn" loc "%s cannot capture %s: it is a dyn, and the collector finds its \ - roots by frame — a copy inside the environment %s is handed would \ - be a live value nothing walks. Pass it in as a parameter, or hold \ - it in a global" - what name what; + roots by frame — a copy inside the environment would be a live \ + value nothing walks. Pass it in as a parameter, or hold it in a \ + global" + what name; let slot = bind ctx name outer.bty ~assignable:false in ctx.caught <- ctx.caught @ [ (name, (outer, slot)) ]; Some { slot; bty = outer.bty; assignable = false; bwhat = None } @@ -1724,7 +1729,24 @@ let rec bind_ty subst (pat : Types.t) (arg : Types.t) = | Types.Array (n, p), Types.Array (m, a) -> Int64.equal n m && bind_ty subst p a | Types.Map (k, v), Types.Map (k', v') -> bind_ty subst k k' && bind_ty subst v v' - | Types.Fn (ps, r), Types.Fn (ps', r') -> + (* Both function types, and the widening between them. + [(Fn [$t] $t)] against a [(CFn [i32] i32)] is the shape every caller of + a generic higher-order function now has, because a [defn]'s name carries + [CFn]: [(apply2 bump 1)]. It has to bind here, where the variables are + decided, and not only in [expect] — [generic_call] binds first and would + have reported the mismatch before [expect] was ever reached, which is + how this arrived as a regression against a program that used to compile. + + The prelude hides it: its higher-order functions bind [$t] from an + earlier argument, so [subst_ty] has already made the parameter concrete + by the time this sees it and [(map-in-place s double)] never took this + path. That is why the corpus stayed green over a real break. + + One way, as everywhere else: a [CFn] pattern does not admit an [Fn] + argument. *) + | Types.Fn (ps, r), Types.Fn (ps', r') + | Types.CFn (ps, r), Types.CFn (ps', r') + | Types.Fn (ps, r), Types.CFn (ps', r') -> List.length ps = List.length ps' && List.for_all2 (bind_ty subst) ps ps' && bind_ty subst r r' (* Nothing generic left on the pattern side: this is ordinary type @@ -2698,30 +2720,59 @@ let unbox_option ctx loc (t : Types.t) (got : Tast.expr) : Tast.expr = compares the signature at the call and a spare argument is a trap. Per *signature* and not per name, so a program pays one small function per - distinct shape it widens rather than one per function it widens. The name - is derived from the type, so two widenings of the same shape share it and - the memo below finds it — the same arrangement [struct_key_pair] uses for - a map's hash and equality pair, and for the same reason. + distinct shape it widens rather than one per function it widens. Two + widenings of the same shape share a thunk, which is what the memo below is + for — the same arrangement [struct_key_pair] uses for a map's hash and + equality pair, and for the same reason. + + **The memo is keyed on the types and the symbol is a counter**, and this + is not a matter of taste. [mangle_ty] flattens a whole signature into one + hyphen-joined string, which loses arity and every type boundary with it: + [(CFn [(Ptr i32)] i32)] and [(CFn [ptr i32] i32)] — the second over a + struct someone called [ptr] — both flatten to [cfn-ptr-i32-to-i32]. Keyed + on that string, the second widening silently reuses the first's thunk and + calls it with the wrong arity, which is a miscompile on both backends and + not a refusal anywhere. The types are the key, compared with + [Types.equal], and nothing is derived from a name. + + ([mangle_ty]'s ambiguity is older than this and is still there for the + generic instantiation names it was written for. At one type's granularity + it is hard to reach; at a whole signature's it is a line of Flan away.) [fparent] is []: not a name anyone wrote, so [defs] hides it, and a marker the redefinition modules match on to carry a copy of their own. *) let thick_thunk env loc ps r = - let name = "thick/" ^ mangle_ty (Types.CFn (ps, r)) in - if not (List.exists (fun (f : Tast.fn) -> f.Tast.name = name) env.lifted) - then begin + let same (f : Tast.fn) = + f.Tast.fparent = Some "" + && List.length f.Tast.params = List.length ps + && List.for_all2 Types.equal f.Tast.params ps + && Types.equal f.Tast.ret r + in + match List.find_opt same env.lifted with + | Some f -> f.Tast.name + | None -> let n = List.length ps in let fty = Types.CFn (ps, r) in let args = List.mapi (fun i t -> mk loc t (Tast.Local i)) ps in let callee = mk loc fty (Tast.Local n) in + (* Counted over the thunks already minted, which is a fact about this + compilation and not about the signature — so no two of them can share + a name however the types are spelled. *) + let name = + Printf.sprintf "thick/%d" + (List.length + (List.filter + (fun (f : Tast.fn) -> f.Tast.fparent = Some "") + env.lifted)) + in env.lifted <- { Tast.name; params = ps; slots = Array.of_list (ps @ [ fty ]); snames = Array.make (n + 1) None; ret = r; body = [ mk loc r (Tast.CallPtr (callee, args)) ]; fdefers = []; fenv = Some n; fparent = Some ""; floc = loc } - :: env.lifted - end; - name + :: env.lifted; + name (* The slot a lifted body's environment arrives in, minted when the body did not capture anything and so has none of its own. @@ -8949,7 +9000,8 @@ and generic_call ctx ~want loc name vars pats pret args = | Types.Slice e | Types.Array (_, e) | Types.Ptr e | Types.Vec e | Types.Option e -> mentions v e | Types.Map (k, w) -> mentions v k || mentions v w - | Types.Fn (ps, r) -> List.exists (mentions v) ps || mentions v r + | Types.Fn (ps, r) | Types.CFn (ps, r) -> + List.exists (mentions v) ps || mentions v r | _ -> false in let bound_exactly v = @@ -9099,7 +9151,24 @@ and generic_call ctx ~want loc name vars pats pret args = && Types.widens_to ~from:a.Tast.ty ~into:f -> widen a.Tast.loc f a | _ -> a) - | _ -> a) + (* And the other widening, for the same reason and at the same + moment: a [CFn] argument against an [(Fn [$t] $t)] parameter. + A parameter that still mentioned a variable was checked with no + expectation at all — there was nothing to expect until the + argument had spoken — so [expect] never saw the pair and never + built the value the instance's signature needs. It is built here, + once the binding is final, exactly as the numeric catch-up above + is. + + A *concrete* [Fn] parameter never reaches this: it was checked + with a want in the first pass and [expect] widened it there. *) + | _ -> + (match subst_ty !subst pat, a.Tast.ty with + | Types.Fn (ps, r), Types.CFn (ps', r') + when Types.equal (Types.Fn (ps, r)) (Types.Fn (ps', r')) -> + mk a.Tast.loc (Types.Fn (ps, r)) + (Tast.Thicken (thick_thunk ctx.env a.Tast.loc ps r, a)) + | _ -> a)) pats targs in (* **A type variable is not instantiated at dyn.** Nothing stopped it before: @@ -11576,21 +11645,35 @@ let rec escaping suspects (e : Tast.expr) = | Tast.RestartCase (cs, body) -> escaping suspects body || List.exists (fun (c : Tast.rclause) -> tail c.Tast.rbody) cs - (* Read out of a struct, which for a function value means read out of an - *environment*: that is the one aggregate a capture may be written into - (see [is_env_struct]), and so the one this can be reading. A copy of a - captured function value carries whatever environment the original did, - so it is suspect exactly as the original was — without this, a lifted - body could hand back its copy of a captured value and the result would - arrive at the caller as an ordinary call result, which is to say as - clean. The same goes for a load through a pointer: nothing a - [(Ptr (Fn ...))] can point at is anywhere but a frame, since a global - and a struct field of function type are both refused. *) - | Tast.Field _ | Tast.CaseField _ | Tast.Deref _ -> true - (* The address of a name, and the result of a call. Neither can carry an - environment: the first never did, and the second cannot because a - function that would return one is refused below. *) - | _ -> false) + (* And then the clean list, which is short, closed, and stated as a list + rather than as a default — a *read* of an [Fn] from anywhere is + suspect unless it is one of these. + + It used to be the other way round, with [Field], [CaseField] and + [Deref] named as suspect and everything else clean, and that spelling + had a hole in it exactly where a list like this cannot: an [(at s 0)] + over a slice of [Fn] is a [Prim], so it read as clean while the [Vec] + and pointer spellings of the same thing were refused. Nothing can + write an [Fn] into a slice today, so it was not reachable — but the + header above claims this enumeration is closed, and a default of + [false] is how that claim stops being true without anyone noticing. + + What is clean, and why. The address of a name never carried an + environment. A widening carries a thunk and a code pointer, which is + not a frame address. And the result of a call cannot carry one, + because a function that would return one is refused below — that + refusal is what this arm rests on, which is why the two have to be + read together. *) + | Tast.FnAddr _ | Tast.Thicken _ | Tast.Call _ | Tast.CallPtr _ -> false + (* Everything else that can produce an [Fn]: read out of a struct — which + for a function value means read out of an *environment*, the one + aggregate a capture may be written into — out of a case, through a + pointer, or out of a container. A copy of a captured function value + carries whatever environment the original did, so it is suspect + exactly as the original was: without this, a lifted body could hand + back its copy of a captured value and the result would arrive at the + caller as an ordinary call result, which is to say as clean. *) + | _ -> true) | _ -> false (* The environment struct a capture built, which is the one aggregate a @@ -11619,7 +11702,6 @@ let escape_check (fn : Tast.fn) = in match e.Tast.e with | Tast.Closure _ -> true - | Tast.Local _ | Tast.Field _ | Tast.CaseField _ | Tast.Deref _ -> false | Tast.If (_, a, b) -> written_here a && written_here b | Tast.Do body | Tast.Let (_, body) | Tast.Handled (_, body) | Tast.WithAlloc (_, body) -> tail body @@ -11628,7 +11710,13 @@ let escape_check (fn : Tast.fn) = | Tast.RestartCase (cs, body) -> written_here body && List.for_all (fun (c : Tast.rclause) -> tail c.Tast.rbody) cs - | _ -> true + (* Everything else is a *read* of a value made elsewhere — a local, a + field, an index, a load through a pointer — and the message that fits + one is the other message, about a value this definition did not make + and cannot see into. The literal is the short list here, exactly as + the clean set is the short list in [escaping]; whichever is short is + the one to write out. *) + | _ -> false in let refuse (e : Tast.expr) where = if not (written_here e) then diff --git a/lib/session.ml b/lib/session.ml index 73ecd749..f6ea36f2 100644 --- a/lib/session.ml +++ b/lib/session.ml @@ -282,8 +282,16 @@ let compatible ?(origin = fun _ -> None) ?(relaxed = []) ~loc refusal is about a function the source does not name." gname ) in + (* The parameters and the return as a [defn] writes them, and not + as [(Fn [...] ...)]. That spelling was harmless while [Fn] was + the only function type and is not now: it is a real type, it is + not the same as [(CFn [...] ...)], and a *declaration* is + neither of them — rendering one as a type invites a reader to + go looking for which of the two this function's name carries, + which is a question about taking its address and not about the + edit that was refused. *) fail loc - "%s changes signature, from (Fn [%s] %s) to (Fn [%s] %s).%s \ + "%s changes signature, from [%s] %s to [%s] %s.%s \ Restart to change it." what (String.concat " " (List.map Types.to_string g.Tast.params)) diff --git a/lib/x86.ml b/lib/x86.ml index 2c4dc4c8..b1c52d1c 100644 --- a/lib/x86.ml +++ b/lib/x86.ml @@ -1505,11 +1505,6 @@ type arg = | Aflt of loc * Types.t | Aptr of loc | Alen of loc - (* A null pointer, which is what a call site with no environment to pass - hands over — every Flan signature takes one. It is its own case rather - than a frame temporary holding zero because there is nothing to spill: - the register is zeroed where it is placed. *) - | Anull (* The C boundary, and the one place this backend must match SysV rather than pick. [check.ml] rejects an aggregate in a [declare] signature and the shim @@ -1558,7 +1553,6 @@ let emit_args f (args : arg list) = | Alen l -> load_int f.b ~dst:reg ~mm:(lmem f (shift l 8) ~scratch:r11) ~size:8 ~signed:true - | Anull -> xor_rr f.b ~dst:reg ~src:reg in (* The stack half first, because it uses rax as its courier and a register argument must not already be sitting in rax while that happens. *) diff --git a/test/programs/fn-escape-at.flan b/test/programs/fn-escape-at.flan new file mode 100644 index 00000000..33e7816e --- /dev/null +++ b/test/programs/fn-escape-at.flan @@ -0,0 +1,13 @@ +;; An index read is a read, and the escape check's clean list has to be a +;; list. A function value out of a slice is refused exactly as one out of a +;; Vec, a struct or a pointer is — the four are the same act and there is no +;; reason for a reader to have to remember which spellings were enumerated. +;; +;; Not reachable today: nothing can write an Fn into a slice, because every +;; position that would have to hold one is refused. It is here so that the +;; day one can, this is already true — the alternative was a default of +;; "clean" for anything the enumeration had not thought of, which is how a +;; closed list quietly stops being closed. +(defn leak [s [(Fn [] i32)]] (Fn [] i32) (at s 0)) + +(defn main [] i32 0) diff --git a/test/programs/fn-generic.flan b/test/programs/fn-generic.flan new file mode 100644 index 00000000..6505a72c --- /dev/null +++ b/test/programs/fn-generic.flan @@ -0,0 +1,33 @@ +;; A generic whose function parameter binds the type variable, which is the +;; shape a name now has to reach: a defn's address carries (CFn [i32] i32), +;; and (apply2 bump 1) has to bind $t from it and then widen the argument. +;; +;; Both halves are here because they fail apart. The binding is Check's +;; bind_ty, which runs inside generic_call and decides the instantiation; the +;; widening is the catch-up pass at the end of the same function, because a +;; parameter that still mentioned a variable was checked with no expectation +;; at all and expect never saw the pair. Get the first without the second and +;; the call site hands one word to an instance that declares two. +;; +;; The prelude does not cover this and that is worth saying: its higher-order +;; functions bind $t from an *earlier* argument, so the parameter is already +;; concrete by the time the function value is reached, and (map-in-place s +;; double) never walks this path at all. +;; +;; The CFn half is new rather than restored: a matching bare address against +;; a (CFn [$t] $t) parameter had no arm either, and fell through to plain +;; equality. + +(defn apply2 [f (Fn [$t] $t) x $t] $t (f (f x))) +(defn applyc [f (CFn [$t] $t) x $t] $t (f (f x))) + +(defn bump [n i32] i32 (+ n 1)) +(defn twice [x f64] f64 (* x 2.0)) + +(defn main [] i32 + (println (apply2 bump 1)) + (println (applyc bump 1)) + ;; A second instantiation, so the two copies are really two and the thunk + ;; the first one minted is not reused at the wrong signature. + (println (apply2 twice 1.5)) + 0) diff --git a/test/programs/fn-thunk-share.flan b/test/programs/fn-thunk-share.flan new file mode 100644 index 00000000..946ed730 --- /dev/null +++ b/test/programs/fn-thunk-share.flan @@ -0,0 +1,25 @@ +;; The widening thunk is memoised per signature, and the key is the types. +;; +;; It used to be a *name*, derived from mangle_ty, which flattens a whole +;; signature into one hyphen-joined string and loses arity and every type +;; boundary with it: (CFn [(Ptr i32)] i32) and (CFn [ptr i32] i32) — the +;; second over a struct someone called ptr — both flatten to the same thing. +;; Keyed on that, the second widening reuses the first's thunk and calls it +;; with the wrong arity, which both backends compile without a word and +;; neither runs. This program is that pair, and it prints 5 and 17. +;; +;; A struct named ptr is legal and ordinary; nothing about the collision +;; needed a program written to provoke it, only two signatures that happened +;; to flatten alike. +(defstruct ptr [a i32 b i32]) + +(defn f1 [p (Ptr i32)] i32 (deref p)) +(defn f2 [a ptr b i32] i32 (+ (.a a) b)) + +(defn use1 [f (Fn [(Ptr i32)] i32)] i32 (let [x 5] (f (addr x)))) +(defn use2 [f (Fn [ptr i32] i32)] i32 (f (ptr {.a 10 .b 0}) 7)) + +(defn main [] i32 + (println (use1 f1)) + (println (use2 f2)) + 0) diff --git a/test/test_acceptance.ml b/test/test_acceptance.ml index 51a8864e..fc9083ea 100644 --- a/test/test_acceptance.ml +++ b/test/test_acceptance.ml @@ -3964,6 +3964,32 @@ level "1" the address and the thunk for the call, and the two have to compose. *) outputs ~dev:true "the two function types, dev" "programs/fn-cfn.flan" fn_ptr_out; + + (* Two signatures that flatten to one string under [mangle_ty], which is + how the thunk memo used to be keyed. Keyed on the name, the second + widening reuses the first's thunk at the wrong arity — a miscompile + both backends emit without a word. Keyed on the types, this prints + 5 and 17. *) + outputs "two signatures that mangle alike" "programs/fn-thunk-share.flan" + "5\n17\n"; + outputs ~opt:"-O0" "two signatures that mangle alike, -O0" + "programs/fn-thunk-share.flan" "5\n17\n"; + + (* A generic whose *function* parameter binds the type variable, which is + the shape a bare name has to reach now that a defn's address carries + CFn. It fails in two places and they fail apart: the binding, in + bind_ty, and the widening, in generic_call's catch-up pass — a + parameter that still mentioned a variable was checked with no + expectation, so expect never saw the pair. The prelude misses it + entirely, because its higher-order functions bind $t from an earlier + argument and the parameter is concrete by the time the function value + is reached. *) + let fn_generic_out = "3\n3\n6\n" in + outputs "a generic that binds its variable through a function type" + "programs/fn-generic.flan" fn_generic_out; + outputs ~opt:"-O0" + "a generic that binds its variable through a function type, -O0" + "programs/fn-generic.flan" fn_generic_out; outputs ~dev:true "an fn capturing by value, dev" "programs/fn-capture.flan" fn_capture_out; @@ -3985,6 +4011,8 @@ level "1" value read back out of an environment is a copy of something that may carry one, and a handler-bind is an expression whose value is its body's — so both are ways for a suspect to be a function's answer. *) + refuses "an index read is a read like any other" + "programs/fn-escape-at.flan" "may carry an environment"; refuses "a captured function value cannot be handed back" "programs/fn-escape-copy.flan" "a return would outlive the frame"; refuses "a handler-bind's value is a return too"