Skip to content

Commit 5dadbf4

Browse files
Stack traces with line numbers
Errors now show the source position of every call site in the trace: Error: inner exploded with x=42 at inner (6:12) at middle (10:12) at outer (13:1) Previously: just function names. The (line:col) suffix reveals which specific call dispatched into each frame — what most languages give you and what was the next debugging bottleneck after stack traces themselves landed in Phase 3. Architecture: position info is attached only to Expression::Call (one new field, `pos: Pos`). Other expressions inherit their frame's call site for error context — same coverage every mainstream interpreter provides at this level. Pos moved from parser.rs to ast.rs and re-exported, so AST nodes can carry positions without depending on parser internals. Tree-walk path: invoke_user_function gained an _at variant that takes the call site; call_function_at threads pos from Expression::Call through dispatch. The frame stack is now Vec<(String, Pos)>. VM path: CompiledFunction carries a parallel Vec<Pos> of op_positions (same length as ops, padded with Pos::unknown). Op layout is unchanged — keeps the optimizer / inline-cache / disasm machinery intact. The compiler's emit_at(op, pos) records the position on emission. Op::Call dispatch consults op_positions[ip-1] and stashes it on the Vm via a `next_call_site` side-channel; run_function reads it on entry, immediately resets so child frames don't inherit. Synthesized calls (heal-pass safe_divide rewrites, parser-generated arr_new in the `[N]name` shorthand, the special-cased res()/fold() keyword forms) carry Pos::unknown() and render with no suffix — keeps trace lines clean when the call wasn't user-written. 42/42 functional examples produce identical output under tree-walk and VM. 92/92 unit tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 3a14ffb commit 5dadbf4

7 files changed

Lines changed: 190 additions & 59 deletions

File tree

omnimcode-core/src/ast.rs

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,32 @@
11
// src/ast.rs - Abstract syntax tree definitions
22

3+
/// Source position. 1-indexed for human-friendly error reports.
4+
/// Lives in ast.rs (rather than parser.rs) so AST nodes can carry
5+
/// positions without depending on parser internals.
6+
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
7+
pub struct Pos {
8+
pub line: u32,
9+
pub col: u32,
10+
}
11+
12+
impl Pos {
13+
/// Sentinel for synthesized AST nodes that don't trace back to
14+
/// a real source location (e.g. nodes created by the heal pass).
15+
pub fn unknown() -> Self {
16+
Pos { line: 0, col: 0 }
17+
}
18+
}
19+
20+
impl std::fmt::Display for Pos {
21+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
22+
if self.line == 0 {
23+
write!(f, "<unknown>")
24+
} else {
25+
write!(f, "{}:{}", self.line, self.col)
26+
}
27+
}
28+
}
29+
330
#[derive(Clone, Debug, PartialEq)]
431
pub enum Statement {
532
Print(Expression),
@@ -170,10 +197,13 @@ pub enum Expression {
170197
Shl(Box<Expression>, Box<Expression>),
171198
Shr(Box<Expression>, Box<Expression>),
172199

173-
// Function call
200+
// Function call. `pos` is the source position of the callee
201+
// identifier — used for stack-trace line numbers. Synthesized
202+
// calls (e.g. from the heal pass) use Pos::unknown().
174203
Call {
175204
name: String,
176205
args: Vec<Expression>,
206+
pos: Pos,
177207
},
178208

179209
// Harmonic operations

omnimcode-core/src/bytecode.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,15 @@ pub struct CompiledFunction {
199199
/// borrow (typical for monomorphic ICs). Cell<u8> is Copy + Clone so
200200
/// the surrounding struct stays cleanly cloneable.
201201
pub call_cache: Vec<std::cell::Cell<u8>>,
202+
/// Source position of each op (for stack-trace line numbers).
203+
/// Same length as `ops`; entries default to Pos::unknown() for
204+
/// ops that don't trace back to user source (e.g. compiler-
205+
/// synthesized arr_new initializers, fall-through nulls). The
206+
/// VM consults this when pushing a call frame so VM-thrown
207+
/// errors get the same "(line:col)" suffix the tree-walk side
208+
/// produces. Cell<()> would suffice but Pos is Copy so a plain
209+
/// Vec works.
210+
pub op_positions: Vec<crate::ast::Pos>,
202211
}
203212

204213
/// A compiled module / program.
@@ -226,6 +235,7 @@ impl Default for Module {
226235
ops: Vec::new(),
227236
constants: Vec::new(),
228237
call_cache: Vec::new(),
238+
op_positions: Vec::new(),
229239
},
230240
functions: std::collections::HashMap::new(),
231241
lambda_asts: Vec::new(),

omnimcode-core/src/compiler.rs

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ pub struct Compiler {
4848
/// existing call_first_class_function dispatches by name through
4949
/// the interpreter (tree-walk), not through module.functions.
5050
pending_lambda_asts: Vec<(String, Vec<String>, Vec<Statement>)>,
51+
/// Source position attached to each emitted op, indexed by op
52+
/// position. Built up alongside `ops`; finish() resizes to match
53+
/// the final op count, padding any missing tail with Pos::unknown().
54+
op_positions: Vec<crate::ast::Pos>,
5155
}
5256

5357
impl Compiler {
@@ -61,6 +65,7 @@ impl Compiler {
6165
fn_return_types: std::collections::HashMap::new(),
6266
pending_lambdas: Vec::new(),
6367
pending_lambda_asts: Vec::new(),
68+
op_positions: Vec::new(),
6469
}
6570
}
6671

@@ -74,6 +79,7 @@ impl Compiler {
7479
fn_return_types: std::collections::HashMap::new(),
7580
pending_lambdas: Vec::new(),
7681
pending_lambda_asts: Vec::new(),
82+
op_positions: Vec::new(),
7783
}
7884
}
7985

@@ -210,8 +216,17 @@ impl Compiler {
210216
}
211217

212218
fn emit(&mut self, op: Op) -> usize {
219+
self.emit_at(op, crate::ast::Pos::unknown())
220+
}
221+
222+
/// Emit an op with an attached source position. Used by Op::Call
223+
/// emission so VM-thrown errors can point back at the call site
224+
/// in the original source.
225+
fn emit_at(&mut self, op: Op, pos: crate::ast::Pos) -> usize {
213226
let idx = self.ops.len();
214227
self.ops.push(op);
228+
// Keep op_positions in lockstep so the VM can index either.
229+
self.op_positions.push(pos);
215230
idx
216231
}
217232

@@ -396,7 +411,11 @@ impl Compiler {
396411
self.compile_expr(e)?;
397412
self.emit(Op::Fold1);
398413
}
399-
Expression::Call { name, args } => {
414+
Expression::Call { name, args, pos } => {
415+
// Capture the call-site position for the bytecode emit
416+
// so VM-thrown errors can show the same "(line:col)" the
417+
// tree-walk side does. Stored on Op::Call directly.
418+
let _site_pos = *pos;
400419
// Mutating built-ins must be specialized so the VM doesn't
401420
// route them through vm_call_builtin's synthetic-arg shim
402421
// (which would otherwise lose the mutation — the shim
@@ -512,7 +531,9 @@ impl Compiler {
512531
for arg in args {
513532
self.compile_expr(arg)?;
514533
}
515-
self.emit(Op::Call(name.clone(), args.len()));
534+
// emit_at attaches the call-site pos so VM-thrown
535+
// errors surface a line number in stack traces.
536+
self.emit_at(Op::Call(name.clone(), args.len()), _site_pos);
516537
}
517538
Expression::Safe(inner) => {
518539
// H.5 host-level: lower `safe <expr>` to the matching
@@ -533,13 +554,13 @@ impl Compiler {
533554
self.compile_expr(r)?;
534555
self.emit(Op::Call("safe_divide".to_string(), 2));
535556
}
536-
Expression::Call { name, args } if name == "arr_get" && args.len() == 2 => {
557+
Expression::Call { name, args, .. } if name == "arr_get" && args.len() == 2 => {
537558
for arg in args {
538559
self.compile_expr(arg)?;
539560
}
540561
self.emit(Op::Call("safe_arr_get".to_string(), 2));
541562
}
542-
Expression::Call { name, args } if name == "arr_set" && args.len() == 3 => {
563+
Expression::Call { name, args, .. } if name == "arr_set" && args.len() == 3 => {
543564
// H.5.2: bare-VAR first arg → emit SafeArrSetNamed
544565
// so the mutation propagates back through VM scope.
545566
// Non-VAR shapes (e.g. nested array) fall through
@@ -910,6 +931,14 @@ impl Compiler {
910931
// Pre-size the inline call cache to match the op count. All slots
911932
// start uncached (0); the VM fills them in on first execution.
912933
call_cache: (0..n).map(|_| std::cell::Cell::new(0u8)).collect(),
934+
// Pad op_positions to match. Compiler appends the correct
935+
// pos at every emit site that knows it (Op::Call); other
936+
// ops get Pos::unknown() and never appear in traces.
937+
op_positions: {
938+
let mut v = self.op_positions;
939+
v.resize(n, crate::ast::Pos::unknown());
940+
v
941+
},
913942
}
914943
}
915944
}

omnimcode-core/src/formatter.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ fn format_expr(expr: &Expression, out: &mut String) {
302302
}
303303
Expression::Shl(l, r) => format_binop(l, "<<", r, out),
304304
Expression::Shr(l, r) => format_binop(l, ">>", r, out),
305-
Expression::Call { name, args } => {
305+
Expression::Call { name, args, .. } => {
306306
out.push_str(name);
307307
out.push('(');
308308
for (i, a) in args.iter().enumerate() {

omnimcode-core/src/interpreter.rs

Lines changed: 79 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,12 @@ pub struct Interpreter {
3535
/// reason as test_failures: a plain OMC global wouldn't propagate
3636
/// to nested assertion calls.
3737
test_current_name: std::cell::RefCell<String>,
38-
/// Names of currently-executing user functions, innermost-last.
39-
/// Pushed at the entry of invoke_user_function, popped on exit
40-
/// (success OR failure). Used to render call traces on errors —
41-
/// future work will also attach source line numbers via per-op
42-
/// metadata in CompiledFunction.
43-
call_stack: Vec<String>,
38+
/// (Function name, call-site position) for currently-executing
39+
/// user functions, innermost-last. The position is the line of
40+
/// the SITE where this fn was called from — that's what the user
41+
/// sees in stack traces. The fn's own internal line numbers don't
42+
/// belong here; they'd need per-statement position tracking.
43+
call_stack: Vec<(String, crate::ast::Pos)>,
4444
}
4545

4646
impl Interpreter {
@@ -394,11 +394,12 @@ impl Interpreter {
394394
return Expression::Call {
395395
name: "safe_divide".to_string(),
396396
args: vec![l, r],
397+
pos: crate::ast::Pos::unknown(),
397398
};
398399
}
399400
Expression::Div(Box::new(l), Box::new(r))
400401
}
401-
Expression::Call { name, args } => {
402+
Expression::Call { name, args, pos } => {
402403
// Typo check at call site. Prefer user-defined fns
403404
// (arities.keys()) over builtins as tiebreaker — a typo
404405
// is more likely meant for a user fn than a builtin.
@@ -436,7 +437,11 @@ impl Interpreter {
436437
healed_args.truncate(expected);
437438
}
438439
}
439-
Expression::Call { name: healed_name, args: healed_args }
440+
// Preserve the original source position through the
441+
// heal pass — we don't reposition synthesized call
442+
// nodes, but we DO keep the original pos so traces
443+
// still point at the user's code.
444+
Expression::Call { name: healed_name, args: healed_args, pos }
440445
}
441446
// Recursive walk for the rest of the structural expressions.
442447
Expression::Add(l, r) => Expression::Add(
@@ -966,8 +971,8 @@ impl Interpreter {
966971
let rv = self.eval_expr(r)?.to_int();
967972
Ok(Value::HInt(HInt::new(lv.wrapping_shr((rv & 63) as u32))))
968973
}
969-
Expression::Call { name, args } => {
970-
self.call_function(name, args)
974+
Expression::Call { name, args, pos } => {
975+
self.call_function_at(name, args, *pos)
971976
}
972977
Expression::Resonance(e) => {
973978
// Match the call_function("res", ...) path: return HFloat resonance score.
@@ -1009,10 +1014,10 @@ impl Interpreter {
10091014
let args = vec![(**l).clone(), (**r).clone()];
10101015
self.call_function("safe_divide", &args)
10111016
}
1012-
Expression::Call { name, args } if name == "arr_get" && args.len() == 2 => {
1017+
Expression::Call { name, args, .. } if name == "arr_get" && args.len() == 2 => {
10131018
self.call_function("safe_arr_get", args)
10141019
}
1015-
Expression::Call { name, args } if name == "arr_set" && args.len() == 3 => {
1020+
Expression::Call { name, args, .. } if name == "arr_set" && args.len() == 3 => {
10161021
self.call_function("safe_arr_set", args)
10171022
}
10181023
_ => self.eval_expr(inner),
@@ -1171,6 +1176,22 @@ impl Interpreter {
11711176
result
11721177
}
11731178

1179+
/// Position-tagged variant — the call site's source position
1180+
/// becomes the line attached to the new stack frame.
1181+
fn call_function_at(
1182+
&mut self,
1183+
name: &str,
1184+
args: &[Expression],
1185+
pos: crate::ast::Pos,
1186+
) -> Result<Value, String> {
1187+
if let Some((params, body)) = self.functions.get(name).cloned() {
1188+
return self.invoke_user_function_at(name, &params, &body, args, pos);
1189+
}
1190+
// Module-qualified calls and builtins don't push frames, so
1191+
// pos doesn't matter — fall through to the unpositioned path.
1192+
self.call_function(name, args)
1193+
}
1194+
11741195
fn call_function(&mut self, name: &str, args: &[Expression]) -> Result<Value, String> {
11751196
// Aliased imports register functions as literal "module.fname"
11761197
// keys in self.functions. Check that BEFORE the dot-split below,
@@ -3347,6 +3368,19 @@ impl Interpreter {
33473368
params: &[String],
33483369
body: &[Statement],
33493370
args: &[Expression],
3371+
) -> Result<Value, String> {
3372+
// Convenience for call sites we haven't position-tagged yet
3373+
// (HOFs, reflective dispatch, module imports).
3374+
self.invoke_user_function_at(name, params, body, args, crate::ast::Pos::unknown())
3375+
}
3376+
3377+
fn invoke_user_function_at(
3378+
&mut self,
3379+
name: &str,
3380+
params: &[String],
3381+
body: &[Statement],
3382+
args: &[Expression],
3383+
call_site: crate::ast::Pos,
33503384
) -> Result<Value, String> {
33513385
let mut eval_args = Vec::new();
33523386
for arg in args {
@@ -3370,7 +3404,7 @@ impl Interpreter {
33703404
// Push a call-stack frame so error messages can show
33713405
// who-called-whom. The frame is popped in BOTH the success
33723406
// and error paths so the trace doesn't leak across calls.
3373-
self.call_stack.push(name.to_string());
3407+
self.call_stack.push((name.to_string(), call_site));
33743408

33753409
let mut exec_err: Option<String> = None;
33763410
for stmt in body {
@@ -3387,11 +3421,15 @@ impl Interpreter {
33873421
self.locals.pop();
33883422

33893423
if let Some(e) = exec_err {
3390-
// Append our own frame and rethrow. Each invoke_user_function
3391-
// up the stack does the same, so the final message lists
3392-
// every frame innermost-first. Mirrors VM run_function's
3393-
// error-path formatting.
3394-
return Err(format!("{}\n at {}", e, display_frame_name(name)));
3424+
// Append our own frame + the call site and rethrow.
3425+
// Each invoke_user_function up the stack does the same,
3426+
// so the final message lists every frame innermost-first.
3427+
return Err(format!(
3428+
"{}\n at {}{}",
3429+
e,
3430+
display_frame_name(name),
3431+
format_call_site(call_site),
3432+
));
33953433
}
33963434

33973435
let result = self.return_value.take().unwrap_or(Value::Null);
@@ -3506,11 +3544,11 @@ impl Interpreter {
35063544
self.return_value.take()
35073545
}
35083546

3509-
/// Push a call-stack frame (function name only for now). The VM
3510-
/// calls this at the entry of run_function so error traces work
3511-
/// for VM-dispatched calls too, not just tree-walk.
3512-
pub fn push_call_frame(&mut self, name: &str) {
3513-
self.call_stack.push(name.to_string());
3547+
/// Push a call-stack frame. The VM calls this at the entry of
3548+
/// run_function so error traces work for VM-dispatched calls too.
3549+
/// Pass Pos::unknown() if the call site isn't tracked.
3550+
pub fn push_call_frame(&mut self, name: &str, call_site: crate::ast::Pos) {
3551+
self.call_stack.push((name.to_string(), call_site));
35143552
}
35153553

35163554
/// REPL-facing: evaluate a single expression in the current
@@ -3535,8 +3573,12 @@ impl Interpreter {
35353573
return msg.to_string();
35363574
}
35373575
let mut out = msg.to_string();
3538-
for fname in self.call_stack.iter().rev() {
3539-
out.push_str(&format!("\n at {}", display_frame_name(fname)));
3576+
for (fname, pos) in self.call_stack.iter().rev() {
3577+
out.push_str(&format!(
3578+
"\n at {}{}",
3579+
display_frame_name(fname),
3580+
format_call_site(*pos),
3581+
));
35403582
}
35413583
out
35423584
}
@@ -3864,6 +3906,18 @@ pub fn display_frame_name(name: &str) -> &str {
38643906
}
38653907
}
38663908

3909+
/// Render a call-site position as the `(line:col)` suffix shown
3910+
/// after the frame name in stack traces. Returns the empty string
3911+
/// for synthesized frames (Pos::unknown) so traces stay clean
3912+
/// when the call wasn't position-tagged.
3913+
pub fn format_call_site(p: crate::ast::Pos) -> String {
3914+
if p.line == 0 {
3915+
String::new()
3916+
} else {
3917+
format!(" ({})", p)
3918+
}
3919+
}
3920+
38673921
/// Test whether `pattern` accepts `value`. On success, appends any
38683922
/// `Pattern::Bind(name)` matches into `bindings` (ordered) so the
38693923
/// caller can install them in the arm's scope.

0 commit comments

Comments
 (0)