From b612e01eead61adb4560ba207792086039e74b89 Mon Sep 17 00:00:00 2001 From: Daniel Morris Date: Wed, 13 May 2026 13:29:46 +0100 Subject: [PATCH 1/2] parser: friendly error for `fld` used as binding name `fld=5` used to cascade through the verifier as `ILO-T006 arity mismatch: 'fld' expects 3 or 4 args, got 0` because the parser quietly built a 0-arg call to the fold builtin. Personas reach for `fld` as a natural variable (field/fold/folder) and pay the retry tax on a misleading error. Mirrors the `cnt`/`brk` handling from the original reserved-word work (commit 8928635). Adds an explicit guard in `parse_decl` and `parse_stmt`: when an identifier `fld` is followed by `Eq`, emit ILO-P011 with a rename suggestion (`field`/`folder`) before the builtin call site has a chance to surface the arity cascade. --- src/parser/mod.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index e0e72b1d..a36173c1 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -315,6 +315,19 @@ impl Parser { format!("pick a different name like `{alt}` or `{}`", &word[..1]), )); } + // Builtin `fld` (fold) used as binding name: `fld=5`. Personas reach + // for `fld` as a natural variable (field/fold/folder); the builtin + // collision otherwise surfaces as a misleading ILO-T006 arity error. + if let Some(Token::Ident(name)) = self.peek() + && name == "fld" + && self.token_at(self.pos + 1) == Some(&Token::Eq) + { + return Err(self.error_hint( + "ILO-P011", + "`fld` is reserved for the fold builtin and cannot be used as an identifier".into(), + "pick a different name like `field` or `folder`".into(), + )); + } match self.peek() { Some(Token::Type) => self.parse_type_decl(), Some(Token::Tool) => self.parse_tool_decl(), @@ -801,6 +814,16 @@ impl Parser { self.advance(); // consume "cnt" Ok(Stmt::Continue) } + Some(Token::Ident(name)) + if name == "fld" && self.token_at(self.pos + 1) == Some(&Token::Eq) => + { + Err(self.error_hint( + "ILO-P011", + "`fld` is reserved for the fold builtin and cannot be used as an identifier" + .into(), + "pick a different name like `field` or `folder`".into(), + )) + } Some(Token::Ident(name)) if name == "wh" => { self.advance(); // consume "wh" let condition = self.parse_expr()?; From bfea0ae513e6baf2a9229e20db4118981bcf0947 Mon Sep 17 00:00:00 2001 From: Daniel Morris Date: Wed, 13 May 2026 13:29:53 +0100 Subject: [PATCH 2/2] tests: regression coverage for `fld` reserved-word friendly error Cross-engine regression test pins the new ILO-P011 path under tree, vm, and cranelift for two shapes: a top-level binding inside a function body, and a binding inside a `@i` loop. Each asserts the friendly message, the `field`/`folder` rename hint, and the absence of the old ILO-T006 arity cascade. A sanity case keeps the `fld` builtin itself exercised. Adds a parallel single-line case to `regression_friendly_errors` alongside the existing `cnt`/`brk` entries so the friendly-error suite stays the canonical reserved-word audit. `examples/fld-reserved-rename.ilo` shows the renamed-variable shape an agent should reach for after hitting the error, and doubles as a higher-level engine harness regression case. --- examples/fld-reserved-rename.ilo | 12 +++ tests/regression_fld_reserved.rs | 128 ++++++++++++++++++++++++++++ tests/regression_friendly_errors.rs | 17 ++++ 3 files changed, 157 insertions(+) create mode 100644 examples/fld-reserved-rename.ilo create mode 100644 tests/regression_fld_reserved.rs diff --git a/examples/fld-reserved-rename.ilo b/examples/fld-reserved-rename.ilo new file mode 100644 index 00000000..b1023db8 --- /dev/null +++ b/examples/fld-reserved-rename.ilo @@ -0,0 +1,12 @@ +-- fld is reserved as the fold builtin. Using it as a variable name +-- triggers ILO-P011 with a rename suggestion, not a misleading +-- arity-mismatch cascade from the builtin call site. +-- +-- Correct shape: pick a different name like field or folder. + +countup n:n>n;field=0;@i 0..n{field=+field 1};field + +-- run: countup 5 +-- out: 5 +-- run: countup 10 +-- out: 10 diff --git a/tests/regression_fld_reserved.rs b/tests/regression_fld_reserved.rs new file mode 100644 index 00000000..c81ce8c1 --- /dev/null +++ b/tests/regression_fld_reserved.rs @@ -0,0 +1,128 @@ +// Regression: `fld` used as a binding name must surface the friendly +// ILO-P011 reserved-word error, not a misleading ILO-T006 arity mismatch +// from the fold builtin. Mirrors the `cnt`/`brk` handling from commit +// 8928635. Personas reach for `fld` as a natural variable name (field / +// fold / folder) and previously paid the retry tax on a cryptic error. +// +// The fix lives in the parser, so all engines surface the same error. +// Tests confirm each engine's CLI path emits ILO-P011 with the right +// wording. + +use std::process::Command; + +fn ilo() -> Command { + Command::new(env!("CARGO_BIN_EXE_ilo")) +} + +fn run(engine: &str, src: &str, entry: &str) -> (bool, String) { + let out = ilo() + .args([src, engine, entry]) + .output() + .expect("failed to run ilo"); + ( + out.status.success(), + String::from_utf8_lossy(&out.stderr).into_owned(), + ) +} + +// `fld=5` at top level inside a function body. +const FLD_BINDING_IN_BODY: &str = "f>n;fld=5;fld"; + +fn check_fld_binding(engine: &str) { + let (ok, stderr) = run(engine, FLD_BINDING_IN_BODY, "f"); + assert!(!ok, "engine={engine}: expected parse failure"); + assert!( + stderr.contains("ILO-P011"), + "engine={engine}: missing ILO-P011, stderr={stderr}" + ); + assert!( + stderr.contains("`fld` is reserved for the fold builtin"), + "engine={engine}: missing friendly message, stderr={stderr}" + ); + assert!( + stderr.contains("field") || stderr.contains("folder"), + "engine={engine}: hint should suggest field/folder, stderr={stderr}" + ); + // Should not cascade into the verifier's misleading arity error. + assert!( + !stderr.contains("ILO-T006"), + "engine={engine}: arity cascade leaked, stderr={stderr}" + ); + assert!( + !stderr.contains("arity mismatch"), + "engine={engine}: arity cascade leaked, stderr={stderr}" + ); +} + +#[test] +fn fld_binding_in_body_tree() { + check_fld_binding("--run-tree"); +} + +#[test] +fn fld_binding_in_body_vm() { + check_fld_binding("--run-vm"); +} + +#[test] +#[cfg(feature = "cranelift")] +fn fld_binding_in_body_cranelift() { + check_fld_binding("--run-cranelift"); +} + +// `fld=5` inside a loop body, the natural shape a persona writes when +// accumulating a fold-style value across iterations. +const FLD_BINDING_IN_LOOP: &str = "f a:n>n;@i 0..a{fld=i};1"; + +fn check_fld_binding_loop(engine: &str) { + let (ok, stderr) = run(engine, FLD_BINDING_IN_LOOP, "f"); + assert!(!ok, "engine={engine}: expected parse failure"); + assert!( + stderr.contains("ILO-P011"), + "engine={engine}: missing ILO-P011, stderr={stderr}" + ); + assert!( + stderr.contains("`fld` is reserved for the fold builtin"), + "engine={engine}: missing friendly message, stderr={stderr}" + ); + assert!( + !stderr.contains("ILO-T006"), + "engine={engine}: arity cascade leaked, stderr={stderr}" + ); +} + +#[test] +fn fld_binding_in_loop_tree() { + check_fld_binding_loop("--run-tree"); +} + +#[test] +fn fld_binding_in_loop_vm() { + check_fld_binding_loop("--run-vm"); +} + +#[test] +#[cfg(feature = "cranelift")] +fn fld_binding_in_loop_cranelift() { + check_fld_binding_loop("--run-cranelift"); +} + +// Sanity: `fld` as the fold builtin still works after the fix. +#[test] +fn fld_as_builtin_still_works() { + let out = ilo() + .args([ + "add x:n y:n>n;+x y;f>n;fld add [1 2 3 4] 0", + "--run-tree", + "f", + ]) + .output() + .expect("failed to run ilo"); + assert!( + out.status.success(), + "fld builtin broken: stderr={}", + String::from_utf8_lossy(&out.stderr) + ); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(stdout.contains("10"), "expected 10, got: {stdout}"); +} diff --git a/tests/regression_friendly_errors.rs b/tests/regression_friendly_errors.rs index ef345328..43457aec 100644 --- a/tests/regression_friendly_errors.rs +++ b/tests/regression_friendly_errors.rs @@ -88,6 +88,23 @@ fn friendly_brk_binding() { assert!(!err.contains("ILO-T028"), "cascade leaked: {err}"); } +#[test] +fn friendly_fld_binding() { + let err = run_err("fld=5"); + assert!(err.contains("ILO-P011"), "stderr: {err}"); + assert!( + err.contains("`fld` is reserved for the fold builtin"), + "stderr: {err}" + ); + assert!(err.contains("field"), "hint should suggest `field`: {err}"); + // Should not cascade through to the verifier's misleading arity error. + assert!(!err.contains("ILO-T006"), "arity cascade leaked: {err}"); + assert!( + !err.contains("arity mismatch"), + "arity cascade leaked: {err}" + ); +} + // ---- Underscore mid-identifier ---- #[test]