Skip to content

fix: prefix-binop operand expands known-arity calls#332

Merged
danieljohnmorris merged 3 commits into
mainfrom
fix/wh-prefix-cond
May 17, 2026
Merged

fix: prefix-binop operand expands known-arity calls#332
danieljohnmorris merged 3 commits into
mainfrom
fix/wh-prefix-cond

Conversation

@danieljohnmorris
Copy link
Copy Markdown
Collaborator

Summary

Six-rerun P1 standing: wh >len q 0{...} failed on first attempt across every persona (gis-analyst, nlp-engineer, others), forcing a bind-first or paren retry every time.

Root cause: parse_prefix_binop parsed both operands via parse_operand (atom-only), so >len q 0 parsed as BinOp(>, Ref(len), Ref(q)) and orphaned 0 — fatal in a wh header where the surrounding context immediately expects { and emits ILO-P003.

Fix mirrors the prefix-?? swap from #310: new parse_prefix_binop_operand helper dispatches to parse_call_arg when the ident at the cursor is a known-arity function AND the next token can start another operand. Falls back to parse_operand otherwise, so bare locals that shadow user fn names (&e f{...} where f is a local) keep resolving via Ref instead of expanding into a zero-arg call. The shadow-safety guard is what kept the FizzBuzz regression (vm_and_does_not_clobber_left_operand) green.

Manifesto win: the natural shape parses on first try. No more "wh prefix-cond" retries chewing through tokens.

Repro

Before:

$ echo 'main>n;q=[1,2,3];wh >len q 0{q=tl q};len q' | ilo run /dev/stdin
{"code":"ILO-P003","message":"expected LBrace, got Number(0.0)",...}

After:

$ echo 'main>n;q=[1,2,3];wh >len q 0{q=tl q};len q' | ilo run /dev/stdin
0

What's in the diff

  • parser: prefix-binop operand expands known-arity calls (commit 1) — parse_prefix_binop now delegates each operand to parse_prefix_binop_operand, which opts into parse_call_arg only when the next ident is known-arity AND followed by an operand-startable token. Suppression set (records, field access, zero-arg paren, postfix-bang) mirrors the existing one in parse_call_arg.
  • tests: cross-engine regression for prefix-binop with call operands (commit 2) — tests/regression_prefix_binop_call.rs covers the originating wh >len q 0 shape plus guard form, let-RHS, prefix-ternary, equality, both-operands-as-calls, parenthesised right operand. Pins the bare-local fallback (wh >v 0 keeps working). Tree + VM + Cranelift.
  • examples: wh-prefix-call shapes for the engine harness (commit 3) — examples/wh-prefix-call.ilo runs through examples_engines.rs so the working shapes are in-context examples for future agents and the example harness catches any regression independently of the parser-specific test.

Notable behaviour change

+f g where f is a 1-arity user fn now parses as BinOp(+, Call(f, [g]), <next>) instead of BinOp(+, Ref(f), Ref(g)). The bare-ref shape was never meaningful (you can't add two function references), so this is a clean upgrade — consistent with the same trade ?? made in #310. If a real program breaks it's an easy bind-first rewrite.

Test plan

  • cargo test --release --features cranelift — full suite green
  • New regression_prefix_binop_call tree/VM/Cranelift all pass
  • examples_engines harness picks up the new example
  • Previously-failing vm_and_does_not_clobber_left_operand (FizzBuzz fixture) still green after the shadow-safe guard
  • regression_wh_gt_trap (existing wh >v 0 cases) still green
  • regression_prefix_nil_coalesce (the precedent fix from fix: prefix ?? accepts call expression as value side #310) still green

Follow-ups

None for this PR. The analogous prefix-ternary (?>len q 0 a b) is already covered by the new tests and works because prefix-ternary's condition flows through parse_prefix_binop. parse_prefix_ternary's then/else operands still use parse_operand — that's a separate trade-off worth opening only if a persona hits it.

`wh >len q 0{...}` failed on first attempt for every persona because
`parse_prefix_binop` parsed both operands via `parse_operand` (atom-only).
`>len q 0` became `BinOp(>, Ref(len), Ref(q))`, leaving `0` orphaned and
the surrounding `wh` header tripping ILO-P003.

Add `parse_prefix_binop_operand` that dispatches to `parse_call_arg` when
the ident at the cursor is a known-arity function AND the next token can
start another operand. Falls back to `parse_operand` otherwise, so bare
locals that shadow user fn names (e.g. `&e f{...}` where `f` is a local)
still resolve via `Ref` instead of expanding into a zero-arg call.

Mirrors the prefix-`??` swap from #310 with the added shadow-safe guard
that vm_and_does_not_clobber_left_operand caught.
Covers the originating gis-analyst shape `wh >len q 0{...}` plus guard
form, let-RHS, prefix-ternary, equality, both-operands-as-calls, and the
parenthesised right operand. Pins the bare-local fallback so `wh >v 0`
keeps working (no regression for the existing prefix-binop shape from
wh-gt-condition).

Tree, VM, Cranelift.
In-context demo of `wh >len q 0`, guard form, prefix-ternary, and a
two-list bound. Runs across tree and VM via examples_engines.rs so any
future regression to the prefix-binop-call path gets caught by the
existing example harness, not just the parser-specific regression
file.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 17, 2026

Codecov Report

❌ Patch coverage is 78.12500% with 7 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/parser/mod.rs 78.12% 7 Missing ⚠️

📢 Thoughts on this report? Let us know!

@danieljohnmorris danieljohnmorris merged commit b8af8e2 into main May 17, 2026
4 of 5 checks passed
@danieljohnmorris danieljohnmorris deleted the fix/wh-prefix-cond branch May 17, 2026 08:25
danieljohnmorris added a commit that referenced this pull request May 17, 2026
The cascading parse-error these three tests asserted no longer fires: #332's parse_prefix_binop change (use parse_call_arg for both operands, mirroring #310's NilCoalesce precedent) made the previously-malformed shape parse cleanly. The program kc x:t>n;len x;body k:t>t;k;main>n;kws=["hi" "there"];fld (a:n k:t>n;+a kc body k) kws 0 now returns 7 on all engines. The diagnostic the tests checked is still present in the parser for shapes that do cascade, just not this one.
danieljohnmorris added a commit that referenced this pull request May 17, 2026
fifteen fixes since 0.11.5, all from rerun5/rerun6 personas plus standing asks: ListView foundation (#334), window-text-perf reshape via ListView (#336), inner-flt predicate inlining (#340), double-minus trap ILO-P021 (#331), bare-ident bang silent-nil regression (#324), Cranelift JIT span plumbing (#335), bool-prefix ternary (#330), wh prefix-cond reparse (#332), --run-engine auto-pick main (#329), subcommand helper hyphens+non-ident (#328), runtime error spans (#335), persona-diagnostic batch 3 (#327), rgxall1+ct (#333), single-line body diagnostic (#322 carry), lambda type-var defensive test (#326), N-deep prefix arity error (#339), prefix-minus span column drift (#338), doc-sync (#337).
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