diff --git a/lib/check.ml b/lib/check.ml index 59ba57f..a13ceb0 100644 --- a/lib/check.ml +++ b/lib/check.ml @@ -83,6 +83,11 @@ type ctx = { (* The type of each slot, newest first. A backend needs it to size the frame — nothing else records it, since the IR refers to slots by index. *) mutable slot_tys : Types.t list; + (* The source name of each slot, newest first, parallel to [slot_tys]. + [None] for a slot the checker invented -- see [Tast.fn.snames]. Recorded + here rather than recovered later because this scope list is the only place + that ever knows it. *) + mutable slot_names : string option list; mutable scope : (string * binding) list; (* innermost first *) (* Deferred forms, most recently registered first — which is also the order they run in. At milestone 4 [defer] is function-scoped (see [check_fn]), @@ -111,14 +116,51 @@ type ctx = { owner : string; } -let fresh_slot ctx ty = +(* [?name] is the source name, when there is one. It is optional so that the + several places that allocate a hidden slot say nothing and get [None] -- + a synthesized slot cannot accidentally acquire a name it was never given. *) +let fresh_slot ?name ctx ty = let s = ctx.slots in ctx.slots <- s + 1; ctx.slot_tys <- ty :: ctx.slot_tys; + ctx.slot_names <- name :: ctx.slot_names; s +(* Shadowing is legal -- [(let [v 11] (let [v 22] ...))] is two slots, both + named [v] -- and the debug info has nowhere to put the distinction. Every + [!DILocalVariable] is scoped to the subprogram, because the typed IR has no + block structure for a [!DILexicalBlock] to be built from, so two variables + called [v] land in one flat scope and lldb answers [p v] with whichever it + finds first. Measured, not assumed: it answers with the *outer* one, so it + prints 11 while the body it is stopped in is computing with 22, and the + inner binding is not listed at all. + + That is the one outcome worse than printing [s3]: a name the debugger is + confident about and wrong about. So a repeat of a name already bound in this + function gets a suffix, and both bindings are then visible and unambiguous. + [~] is the reader's delimiter and cannot occur in a source symbol (the same + reason [destructure~nth] is spelled that way), so [v~2] is visibly the + compiler's doing and can never collide with something the programmer wrote. + + This is a way of not lying, not a way of being right: [v] is still the outer + binding everywhere, including inside the inner one's extent. Scoping the + variables properly means emitting a [!DILexicalBlock] per [Let] and moving + the [llvm.dbg.declare]s out of the entry block to the binding sites, which + needs block structure this IR does not carry. *) let bind ctx name bty ~assignable = - let slot = fresh_slot ctx bty in + let taken n = List.exists (fun s -> s = Some n) ctx.slot_names in + let name' = + if not (taken name) then name + else + let rec go k = + let c = Printf.sprintf "%s~%d" name k in + if taken c then go (k + 1) else c + in + go 2 + in + let slot = fresh_slot ~name:name' ctx bty in + (* [ctx.scope] keeps the *source* name: the suffix is a debug-info artifact + and resolving [v] must still find the innermost binding. *) ctx.scope <- (name, { slot; bty; assignable }) :: ctx.scope; slot @@ -573,7 +615,7 @@ and check_handler_bind ctx ?want loc clauses body = (* Its own context: a fresh frame, an empty scope, and no way to reach the enclosing one. *) let hctx = - { env = ctx.env; ret = Types.Unit; slots = 0; slot_tys = []; + { env = ctx.env; ret = Types.Unit; slots = 0; slot_tys = []; slot_names = []; scope = []; defers = []; outer = ctx.scope; in_handler = true; in_frames = None; in_defer = false; owner = "" } in (* The condition crosses as a pointer, because the handler runs while @@ -608,6 +650,7 @@ and check_handler_bind ctx ?want loc clauses body = ctx.env.lifted <- { Tast.name = fname; params = [ Types.Ptr ty ]; slots = Array.of_list (List.rev hctx.slot_tys); + snames = Array.of_list (List.rev hctx.slot_names); ret = Types.Unit; body = hbody; fdefers = []; fparent = Some ctx.owner; floc = c.Ast.hloc } :: ctx.env.lifted; @@ -1456,7 +1499,7 @@ let collect env (decls : Ast.decl list) = not check once no progress is left has a real error, so the last round is run without swallowing it. *) let infer (_, v) = - (check { env; ret = Types.Unit; slots = 0; slot_tys = []; scope = []; defers = []; + (check { env; ret = Types.Unit; slots = 0; slot_tys = []; slot_names = []; scope = []; defers = []; outer = []; in_handler = false; in_frames = None; in_defer = false; owner = "" } v).Tast.ty in let pending = ref (List.rev !untyped) in @@ -1509,7 +1552,7 @@ let check_finite env = let check_fn env (fn : Ast.fn) : Tast.fn = let params, ret = Hashtbl.find env.fns fn.Ast.name in - let ctx = { env; ret; slots = 0; slot_tys = []; scope = []; defers = []; + let ctx = { env; ret; slots = 0; slot_tys = []; slot_names = []; scope = []; defers = []; outer = []; in_handler = false; in_frames = None; in_defer = false; owner = fn.Ast.name } in List.iter2 @@ -1579,12 +1622,13 @@ let check_fn env (fn : Ast.fn) : Tast.fn = in { Tast.name = fn.Ast.name; params; slots = Array.of_list (List.rev ctx.slot_tys); + snames = Array.of_list (List.rev ctx.slot_names); (* The same defers again, for the transfer exit path §5 describes. The normal path has them spliced into [body] above. *) ret; body; fdefers = ctx.defers; fparent = None; floc = fn.Ast.nloc } let check_global env (d : Ast.decl) : Tast.global option = - let ctx () = { env; ret = Types.Unit; slots = 0; slot_tys = []; scope = []; defers = []; + let ctx () = { env; ret = Types.Unit; slots = 0; slot_tys = []; slot_names = []; scope = []; defers = []; outer = []; in_handler = false; in_frames = None; in_defer = false; owner = "" } in match d.Ast.d with | Ast.Defvar (n, _, init) -> @@ -1691,10 +1735,12 @@ let program (decls : Ast.decl list) : Tast.program = fst (program_with_env decls (* One expression, checked against a program that is already running. The frame is empty — a REPL expression has no parameters and no enclosing function — so the slots it needs are whatever its own [let]s allocate. *) -let expression env (e : Ast.expr) : Tast.expr * Types.t array = +let expression env (e : Ast.expr) : + Tast.expr * Types.t array * string option array = let ctx = - { env; ret = Types.Unit; slots = 0; slot_tys = []; scope = []; defers = []; + { env; ret = Types.Unit; slots = 0; slot_tys = []; slot_names = []; scope = []; defers = []; outer = []; in_handler = false; in_frames = None; in_defer = false; owner = "" } in let t = check ctx e in - (t, Array.of_list (List.rev ctx.slot_tys)) + (t, Array.of_list (List.rev ctx.slot_tys), + Array.of_list (List.rev ctx.slot_names)) diff --git a/lib/emit.ml b/lib/emit.ml index 3a85c50..3ca0d22 100644 --- a/lib/emit.ml +++ b/lib/emit.ml @@ -1301,20 +1301,30 @@ let signature ~named (fn : Tast.fn) = visibility in a shared object is interposable: [@"flan.bump"] inside the module would resolve to the *host's* copy, so the installer would publish the function it was replacing and the reload would appear to do nothing. *) -(* The name a slot goes into the debug info under. The typed IR refers to - locals by index and nothing records what they were called -- [Check] knows, - in its scope list, and drops it. So a parameter gets the name the source - gave it, recovered by the driver and handed down in [pnames], and everything - else gets [s], which is the slot it actually is. A [let]-bound local - printing as [s4] is a real gap and it is named here rather than papered - over: fixing it means the typed IR carrying the name, which is a change to - [Tast]. *) -let slot_name ~pnames ~nparams i = - if i < nparams then - match List.nth_opt pnames i with - | Some n when n <> "" -> n - | _ -> Printf.sprintf "p%d" i - else Printf.sprintf "s%d" i +(* The name a slot goes into the debug info under. [Tast.fn.snames] carries the + source name of every slot the source named, parameters included, so that is + the answer wherever there is one. + + A slot with no name is one the compiler invented -- [dotimes]'s hidden + bound, the pair (min) and (max) evaluate their operands into -- and it keeps + [s], which is what it actually is. That is deliberate rather than a + fallback: a synthesized slot has no source name to print, and inventing a + plausible one would put a variable in the debugger that the programmer + cannot find in the file. [s4] is honest about being the frame's fourth slot. + + [snames] is indexed defensively because a driver may build a frame by + appending arrays ([Session]'s evaluation thunk does), and a short [snames] + should cost a name, not raise. *) +let slot_name ~pnames ~snames ~nparams i = + let named = if i < Array.length snames then snames.(i) else None in + match named with + | Some n when n <> "" -> n + | _ -> + if i < nparams then + match List.nth_opt pnames i with + | Some n when n <> "" -> n + | _ -> Printf.sprintf "p%d" i + else Printf.sprintf "s%d" i let emit_fn m ?(hidden = false) ?(pnames = []) (fn : Tast.fn) = let n = Array.length fn.Tast.slots in @@ -1372,7 +1382,8 @@ let emit_fn m ?(hidden = false) ?(pnames = []) (fn : Tast.fn) = dnode d (Printf.sprintf "!DILocalVariable(name: \"%s\"%s, scope: !%d, file: !%d, line: %d, type: !%d)" - (dstr (slot_name ~pnames ~nparams i)) arg sub file f.dline + (dstr (slot_name ~pnames ~snames:fn.Tast.snames ~nparams i)) + arg sub file f.dline (dty m d ty))) fn.Tast.slots) in diff --git a/lib/session.ml b/lib/session.ml index 540bd39..92851a7 100644 --- a/lib/session.ml +++ b/lib/session.ml @@ -574,7 +574,7 @@ let eval_expr ?(origin = "") t src : change = | [] -> fail Loc.unknown "nothing to evaluate" | _ :: f :: _ -> fail f.Form.loc "one expression at a time" in - let checked, base = Check.expression t.env (Parse.expr form) in + let checked, base, bnames = Check.expression t.env (Parse.expr form) in let c = { structs = t.program.Tast.structs; enums = Hashtbl.fold (fun k v acc -> (k, v) :: acc) t.env.Check.enums []; @@ -589,7 +589,10 @@ let eval_expr ?(origin = "") t src : change = let name = Printf.sprintf "eval/%d" t.thunks in let thunk : Tast.fn = { Tast.name; params = []; ret = Types.Unit; body; fdefers = []; fparent = None; floc = loc; - slots = Array.append base (Array.of_list (List.rev c.slots)) } + slots = Array.append base (Array.of_list (List.rev c.slots)); + (* The expression's own [let]s keep their names; the slots [render] added + behind them are the walk's own scratch and have none to keep. *) + snames = Array.append bnames (Array.make (List.length c.slots) None) } in (* Built against the program but never spliced into it: an evaluation is not a declaration, and adding one would leave the session carrying an eval/N diff --git a/lib/tast.ml b/lib/tast.ml index 2f686dd..72cb264 100644 --- a/lib/tast.ml +++ b/lib/tast.ml @@ -113,6 +113,15 @@ type fn = { name : string; params : Types.t list; (* bound to slots 0 .. n-1, in order *) slots : Types.t array; (* the frame: one entry per slot *) + (* What the source called each slot, parallel to [slots]. [None] is a slot + the compiler made up and no one wrote a name for -- [dotimes]'s hidden + bound, the pair (min) and (max) evaluate their operands into, the slot a + tail expression goes through. Names are otherwise gone from this IR (see + the header); this is the one exception, and it exists so a debug build can + emit a [!DILocalVariable] that says [lo] where the source said [lo]. A + backend is free to ignore it entirely -- nothing is *resolved* through it, + and a slot is still only ever referred to by index. *) + snames : string option array; ret : Types.t; body : expr list; (* The defers again, innermost first. [body] already has them spliced onto diff --git a/test/test_acceptance.ml b/test/test_acceptance.ml index 21ac026..28f42f7 100644 --- a/test/test_acceptance.ml +++ b/test/test_acceptance.ml @@ -1309,11 +1309,11 @@ ERR@7 unexpected token: not the kind the caller was reading (* ptr+len, and shown as ptr+len — there is no owner and no capacity to hide, so two members are the whole truth about a string. *) ("name: \"string\", size: 128", "string"); - (* A let-bound local has no name to keep: the typed IR refers to - slots by index and [Check] drops what they were called, so it is - emitted as the slot it is. Asserted rather than left implicit, - because this is the one honest gap in the picture. *) - ("!DILocalVariable(name: \"s0\"", "a let-bound local, named by its slot"); + (* A let-bound local carries the name the source gave it. [Tast.fn] + records one per slot and [Check] fills it in at the binding, so + [(let [c ...)] is [c] in the debug info and not [s0] -- which is + what it used to be, and was the one honest gap in this picture. *) + ("!DILocalVariable(name: \"c\"", "a let-bound local, named by its source name"); ("!llvm.dbg.cu = ", "the compile unit is registered"); (* Without this LLVM discards every node above, silently. *) ("!{i32 2, !\"Debug Info Version\", i32 3}", "the module flag") ]; @@ -1341,6 +1341,40 @@ ERR@7 unexpected token: not the kind the caller was reading print_endline "FAIL the transfer channel appeared as a local variable" end; + (* The two rules about a name that is not simply the source's own. + + A slot the compiler invented has no source name and keeps [s]: + [dotimes] evaluates its bound once into a hidden slot, and calling that + something plausible would put a variable in the debugger that is not in + the file. [i] is the programmer's and is named; the bound is not. + + And a shadowed name is disambiguated. Every [!DILocalVariable] is scoped + to the subprogram — the typed IR has no block structure to build a + [!DILexicalBlock] from — so two slots both called [v] leave lldb + answering [p v] with whichever it finds first. Measured: it answers with + the outer one, and does not list the inner at all, so the debugger is + confident and wrong. [~] cannot occur in a source symbol, so [v~2] is + unambiguous and visibly the compiler's. The prelude shadows in + [split-next], so this rule is load-bearing for the library too. *) + let ir = + debug_ir "(defn spin [n i32] i32\n\ + \ (let [v 11]\n\ + \ (let [v 22]\n\ + \ (dotimes [i n] (set v (+ v i)))\n\ + \ v)))\n\ + (defn main [] i32 (spin 3))\n" + in + List.iter + (fun (needle, what) -> + if not (contains ir needle) then begin + incr failures; + Printf.printf "FAIL DWARF for %s\n wanted: %S\n" what needle + end) + [ ("!DILocalVariable(name: \"v\"", "the outer of two shadowed bindings"); + ("!DILocalVariable(name: \"v~2\"", "the inner one, disambiguated"); + ("!DILocalVariable(name: \"i\"", "a dotimes counter, which is the source's"); + ("!DILocalVariable(name: \"s4\"", "dotimes' hidden bound, which is not") ]; + (* LLVM's own verifier, over both entry points. String needles cannot see a DISubprogram the compile unit does not reach, or a call without a !dbg inside a function that has debug info — and that second one is a @@ -1513,7 +1547,13 @@ ERR@7 unexpected token: not the kind the caller was reading value the program put there. *) lldb_case "lldb: breakpoint, frames and locals" "programs/debug.flan" [ "flan.tick"; "at debug.flan:"; "flan.main at debug.flan:"; - "(int) n = 41"; "alive = true"; "heat = 3.25"; "id = 7"; "len = 5" ]; + "(int) n = 41"; "alive = true"; "heat = 3.25"; "id = 7"; "len = 5"; + (* And the let-bound local under its own name rather than [s0], which + is the gap this closes. Only the name is claimed here: a name + breakpoint stops on the function's first line, which is before the + [let] has stored anything, so the value at this point is whatever + the frame happened to hold. The value is pinned just below. *) + "(int) bump" ]; (* And the same, with the fields permuted. If the offsets were not following the declaration, the values would land on the wrong names here and nowhere else. *) @@ -1521,6 +1561,26 @@ ERR@7 unexpected token: not the kind the caller was reading "programs/debug-permuted.flan" [ "at debug-permuted.flan:"; "flan.main at debug-permuted.flan:"; "(int) n = 41"; "alive = true"; "heat = 3.25"; "id = 7"; "len = 5" ]; + (* The value, which the case above deliberately does not claim. Every + [!DILocalVariable] is scoped to the whole subprogram and carries the + function's own line, so a let-bound local is nominally in scope from + entry and reads as garbage until its binding runs. Breaking *after* + the binding is what makes the value load-bearing: [bump] is n+1 and n + is 41, so 42 is the only right answer, and a [!DILocalVariable] + attached to the wrong alloca prints something else. That is the check + that a name which is present is also not a lie. *) + let exe = debug_compile "programs/debug.flan" in + let _, text = + lldb_run exe + [ "breakpoint set --file debug.flan --line 20"; "run"; + "frame variable bump" ] + in + if not (contains text "(int) bump = 42") then begin + incr failures; + print_endline "FAIL lldb: a let-bound local's value after its binding"; + print_endline text + end; + (* A dev build routes every call through a cell, so the call site is an indirect call through a mutable global. The frame above it is still the Flan caller with its own line: the indirection is in how the