Skip to content

parser: reject builtin-named binding LHS with ILO-P011#294

Merged
danieljohnmorris merged 2 commits into
mainfrom
fix/builtin-shadow-flat-freq
May 16, 2026
Merged

parser: reject builtin-named binding LHS with ILO-P011#294
danieljohnmorris merged 2 commits into
mainfrom
fix/builtin-shadow-flat-freq

Conversation

@danieljohnmorris
Copy link
Copy Markdown
Collaborator

Summary

Pattern-matches PR #245's parse-time rejection of builtin-named user functions, extending it to local-binding LHS. A binding like flat=cat ls " " was silently accepted by the parser; any later use of flat in operand position resolved to the 1-arg flatten builtin (the verifier checks is_builtin before locals), and the agent saw the misleading ILO-T006 'flat' expects 1 args, got 0 instead of a signal that the local binding was being shadowed.

Persona origin: 2026-05-16 pdf-analyst re-run against v0.11.2, friction #6.

Manifesto framing: silent mis-dispatch is the worst class of bug for an AI agent because there is no diagnostic to retry against. The parser now intercepts at the binding site with ILO-P011 \flat` is a builtin and cannot be used as a binding name plus a rename hint (myflat, flatv`). First retry is the right one.

Repro

Before:

$ ilo 'main>n;flat=5;flat' --run-tree main
ILO-T006 arity mismatch: 'flat' expects 1 args, got 0

After:

$ ilo 'main>n;flat=5;flat' --run-tree main
ILO-P011 `flat` is a builtin and cannot be used as a binding name
hint: rename to something like `myflat` or `flatv`. Builtins shadow local bindings in call position, so reusing the name silently mis-dispatches.

What's in the diff

  • f98748e parser: reject builtin-named binding LHS with ILO-P011 — adds Builtin::is_builtin(name) guards to both parse_decl (top-level) and parse_stmt (in-function) binding sites, runs after the existing per-name fld= block so the fold-specific hint still wins. Twelve-builtin cross-engine regression test (tree, VM, Cranelift) covering both top-level and in-function binding forms, the exact pdf-analyst rerun3 repro, and a sanity check that the renamed binding compiles and runs. examples/builtin-binding-name-rename.ilo demonstrates the correct shape.

  • 3547b6b docs: note builtin names rejected as binding LHS — extends the Reserved words section in SPEC.md (auto-regenerates ai.txt via build.rs) and the syntax-rules section in skills/ilo/SKILL.md to mention the new rejection, listing the same names the parser checks and both the function-name and binding-LHS error shapes.

Test plan

  • cargo test --release --features cranelift passes the full suite (127 test crates, 0 failures) in the isolated worktree target dir
  • New regression_builtin_binding_name (15 tests) passes on tree, VM, and Cranelift
  • Existing regression_friendly_errors::friendly_fld_binding still passes (per-name fld block preserved)
  • Existing regression_builtin_fn_name (PR reject builtin-named user functions at parse time #245 parent) still passes
  • examples_engines.rs exercises the new builtin-binding-name-rename.ilo across every engine and confirms len parts == 3
  • git diff ai.txt matches the SPEC.md-driven regeneration (build.rs CI check passes)

Follow-ups

  • Destructure pattern names ({flat;y}=...) and @flat xs{...} foreach variables are out of scope. They share the same operand-position dispatch risk but are much rarer in practice and would each need a separate intercept point. Filing as parked.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@danieljohnmorris danieljohnmorris force-pushed the fix/builtin-shadow-flat-freq branch from 3547b6b to 8d7342d Compare May 16, 2026 09:58
A local binding like `flat=cat ls " "` was silently accepted, then any
later use of `flat` in operand position resolved to the 1-arg flatten
builtin (the verifier checks is_builtin before locals), surfacing as a
misleading `ILO-T006 'flat' expects 1 args, got 0`. The agent has no
signal that the local binding is being shadowed.

Mirrors the PR #245 precedent for builtin-named user functions: intercept
at parse time with ILO-P011 and a rename hint (`myflat`, `flatv`) in
both parse_decl (top-level) and parse_stmt (in-function) binding sites.
Existing `fld=` per-name block keeps its specific fold-builtin message
since it runs before the generic check.

Cross-engine regression test covers twelve builtin names across tree, VM,
and Cranelift, plus the exact pdf-analyst rerun3 repro and a sanity check
that the renamed binding compiles and runs. examples/builtin-binding-name-rename.ilo
demonstrates the correct shape (`body=cat ls " "`) for the
`spl body ". "` pipeline.
Extends the Reserved words section in SPEC.md (auto-regenerates ai.txt
via build.rs) and the syntax-rules section in skills/ilo/SKILL.md to
mention the new parse-time rejection of builtin-named bindings. Lists
the same names the parser checks (flat, frq, map, flt, cat, len, srt,
hd, tl, ord, fld, lst) and shows both the function-name and binding-LHS
error shapes so agents see the rename hint shape before they hit it.
@danieljohnmorris danieljohnmorris force-pushed the fix/builtin-shadow-flat-freq branch from 8d7342d to d8acf9e Compare May 16, 2026 10:15
@danieljohnmorris danieljohnmorris merged commit fc6e670 into main May 16, 2026
5 checks passed
@danieljohnmorris danieljohnmorris deleted the fix/builtin-shadow-flat-freq branch May 16, 2026 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant