From 3702a652d6ab7855b903231d9a96f1b301fc4622 Mon Sep 17 00:00:00 2001 From: Ed Date: Sat, 3 Oct 2026 19:17:54 +0300 Subject: [PATCH] fix: prefer cursor fields over same-named local reads --- crates/foxvm/src/compiler.rs | 7 ++-- crates/foxvm/src/vm.rs | 2 ++ crates/foxvm/tests/local_field_precedence.rs | 37 ++++++++++++++++++++ docs/local-field-precedence.md | 25 +++++++++++++ 4 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 crates/foxvm/tests/local_field_precedence.rs create mode 100644 docs/local-field-precedence.md diff --git a/crates/foxvm/src/compiler.rs b/crates/foxvm/src/compiler.rs index 6017356..ace02e4 100644 --- a/crates/foxvm/src/compiler.rs +++ b/crates/foxvm/src/compiler.rs @@ -3786,8 +3786,11 @@ impl ModuleCompiler { fb.emit(Instr::Const(c)); } ExprKind::Var(n) => { - let v = self.var_target(fb, n); - self.load_var(fb, v); + // Even a declared LOCAL can be shadowed by a current-table field. + // Resolve bare expression names at runtime; explicit m.x and the + // compiler's internal slots remain memory-only LoadLocal reads. + let name = self.name(&n.upper); + fb.emit(Instr::LoadName(name)); } ExprKind::This => { fb.emit(Instr::LoadThis); diff --git a/crates/foxvm/src/vm.rs b/crates/foxvm/src/vm.rs index 833073c..585d1eb 100644 --- a/crates/foxvm/src/vm.rs +++ b/crates/foxvm/src/vm.rs @@ -3298,6 +3298,8 @@ impl Vm { FieldRead::Value(v) => fb.stack.push(v), FieldRead::Suspend(req) => return Ok(self.ask(fb, req)), } + } else if let Some(slot) = module.funcs[fb.frames.last().expect("frame").func as usize].locals.iter().position(|local| local == &name) { + fb.stack.push(self.load_local(fb, slot as u32)?); } else if let Some(v) = self.load_name(fb, &name) { fb.stack.push(v); } else if let Some(alias) = self.query_field(fb, &name) { diff --git a/crates/foxvm/tests/local_field_precedence.rs b/crates/foxvm/tests/local_field_precedence.rs new file mode 100644 index 0000000..68085d0 --- /dev/null +++ b/crates/foxvm/tests/local_field_precedence.rs @@ -0,0 +1,37 @@ +use foxvm::mock_host::{MockHost, run_program}; + +#[test] +fn unqualified_field_wins_over_same_named_local() { + let source = "CREATE CURSOR choices (item_id I)\nINSERT INTO choices VALUES (42)\nLOCAL m.item_id\nm.item_id = item_id\nCREATE CURSOR picked (item_id I)\nINSERT INTO picked VALUES (m.item_id)\n? item_id"; + let (_, out) = run_program(source, &mut MockHost::new()).unwrap(); + assert_eq!(out.last().unwrap().trim(), "42"); +} + +fn output(source: &str) -> Vec { + run_program(source, &mut MockHost::new()).unwrap().1.into_iter().map(|s| s.trim().to_string()).collect() +} + +#[test] +fn explicit_memory_reads_and_local_fallback_still_work() { + assert_eq!(output("LOCAL item_id\nm.item_id=7\n? item_id\nCREATE CURSOR choices (item_id I)\nINSERT INTO choices VALUES (42)\n? item_id\n? m.item_id\nUSE IN choices\n? item_id"), ["7", "42", "7", "7"]); +} + +#[test] +fn assignments_do_not_write_the_same_named_field() { + assert_eq!(output("CREATE CURSOR choices (item_id I)\nINSERT INTO choices VALUES (42)\nLOCAL item_id\nitem_id=9\n? item_id\n? m.item_id"), ["42", "9"]); +} + +#[test] +fn parameters_follow_field_precedence_without_leaking_callers_locals() { + assert_eq!(output("LOCAL item_id\nitem_id=8\nCREATE CURSOR choices (item_id I)\nINSERT INTO choices VALUES (42)\nDO show WITH 12\n? m.item_id\nPROCEDURE show\nLPARAMETERS item_id\n? item_id\n? m.item_id\nENDPROC"), ["42", "12", "8"]); +} + +#[test] +fn local_fallback_beats_public_variable() { + assert_eq!(output("PUBLIC amount\namount=99\nDO show\n? amount\nPROCEDURE show\nLOCAL amount\namount=7\n? amount\nENDPROC"), ["7", "99"]); +} + +#[test] +fn dynamic_expression_uses_the_same_field_precedence() { + assert_eq!(output("LOCAL item_id\nitem_id=7\nCREATE CURSOR choices (item_id I)\nINSERT INTO choices VALUES (42)\n? EVALUATE('item_id')\n? EVALUATE('m.item_id')"), ["42", "7"]); +} diff --git a/docs/local-field-precedence.md b/docs/local-field-precedence.md new file mode 100644 index 0000000..e8849da --- /dev/null +++ b/docs/local-field-precedence.md @@ -0,0 +1,25 @@ +# Fields and same-named local variables + +Bare variable expressions now resolve a field in the selected work area before +a same-named LOCAL or LPARAMETERS slot. If there is no matching field, the local +slot still wins over private/public variables. Explicit m.name remains a direct +memory-variable read, and assignment targets still write memory variables. + +Previously a declared local compiled directly to LoadLocal even for an +unqualified expression. A pattern such as LOCAL m.item_id followed by +m.item_id = item_id read the uninitialized local (.F.) instead of the numeric +field. Inserting that value into an integer column then raised error 9. + +The compiler now emits LoadName for bare expression reads; the VM checks fields, +then the current function's local slots, then the existing dynamic-name lookup. +Internal temporary slots and explicit m.name still use LoadLocal. No bytecode +layout changed. Rebuild compiled projects/bundles to obtain the corrected reads. + +Reference: [VFP Accessing Variables](https://www.vfphelp.com/help/html/4b4bccf2-fa78-4377-b838-a94863efb6b2.htm). + +The synthetic regression failed with error 9 before the fix. Six focused tests +cover insertion, explicit m., local fallback, assignments, parameters, public +shadowing and EVALUATE. The full VM suite (680 tests including doctests), 16 +WASM bridge/sample tests, WASM build, typecheck, lint and app build pass on macOS +ARM64. Native Windows VFP was not run. The alternate m-> spelling remains an +existing parser limitation outside this fix.