Skip to content

fix: prefix ?? accepts call expression as value side#310

Merged
danieljohnmorris merged 3 commits into
mainfrom
fix/qq-call-parse
May 16, 2026
Merged

fix: prefix ?? accepts call expression as value side#310
danieljohnmorris merged 3 commits into
mainfrom
fix/qq-call-parse

Conversation

@danieljohnmorris
Copy link
Copy Markdown
Collaborator

Summary

Prefix ?? (nil-coalesce) was parsing its value side with parse_operand, which only consumes an atom. So the most common nil-coalesce shape in agent-written ilo, ??mget m "k" 0, mis-parsed: mget was taken as the value atom and m "k" 0 as the default expression, which then failed because m is a Map, not a function.

Workarounds existed (??(mget m "k") 0, bind-first), but every site cost extra tokens and agent attention. Manifesto framing: this is a parser-precedence tax paid on every map lookup with a fallback, and it can be removed for free with the arity-cap pattern the parser already uses in four other places.

Repro before/after

main>n;m=mset mmap "k" 42;v=??mget m "k" 0;v

Before (main):

{"code":"ILO-T004","message":"undefined variable 'mget'", ...}
{"code":"ILO-T005","message":"'m' is a M _ n, not a function (called with 2 args)", ...}

After (this branch): 42

What's in the diff

Three commits, one logical change each:

  1. parser: prefix ?? accepts call expression as value side — swaps the value-side parser in both prefix-?? arms (parse_expr_inner and parse_operand) from parse_operand() to parse_call_arg(false, None). parse_call_arg is a strict superset: it handles every operand case and additionally expands a known-arity ident into a call consuming exactly its declared arity. This mirrors the existing arity-cap behaviour in no_whitespace_call (list literals) and parse_call_arg's own nested-call block. Identifier-only ??x default, paren ??(expr) default, infix c??d, and chained f a ?? g b ?? d all keep working because parse_call_arg falls through to parse_operand for non-known-arity idents.

  2. test: prefix ?? with call-expression value across all engines — extends tests/regression_prefix_nil_coalesce.rs with four new cases (hit, miss, two-call chain, paren workaround still works) running on tree / VM / Cranelift.

  3. example: prefix ?? with map lookup and defaultexamples/qq-call-default.ilo, pinned by the engine harness. Three -- run: entries (hit, miss, chain) act as both regression coverage and an in-context demo for agents.

Test plan

  • Repro fixed on tree, VM, Cranelift (verified manually before commit)
  • cargo test --release --features cranelift full suite green (3071 + 340 + others = 0 failures)
  • cargo fmt --check clean
  • cargo clippy --release --features cranelift --all-targets -- -D warnings clean
  • Targeted regression file passes 3/3 engine variants
  • Examples harness picks up the new example file and passes
  • Existing workarounds (parens, bind-first) confirmed still working

Follow-ups

None. Adjacent observation: ??-x default (prefix-minus value side) is broken on main too, but that's a separate operand-handling issue and out of scope for this fix.

Both prefix-`??` arms (parse_expr_inner and parse_operand) previously
used parse_operand for the value, which only consumes an atom. So
`??mget m "k" 0` mis-parsed as `??mget (m "k" 0)`: `mget` was the value,
and `m "k" 0` was the default expression, which then failed because
`m` is a Map, not a function.

Switching to parse_call_arg(false, None) reuses the arity-cap pattern
already used in no_whitespace_call and the nested-call block at
parse_call_arg:2193: a known-arity ident consumes exactly its declared
arity, leaving the remainder for the default. Identifier-only
`??x default`, parenthesised `??(expr) default`, and infix `c ?? d` all
keep working because parse_call_arg falls through to parse_operand for
non-known-arity idents.

Bare `??mget m "k" 0` is the most common nil-coalesce site in
agent-written ilo, so removing the parens/bind-first tax pays off on
every map lookup with a fallback.
Adds four cases to the existing prefix-nil-coalesce regression file:
hit (mget returns value), miss (mget returns nil, default fires),
two-call chain (first miss + second hit), and paren workaround
(still works post-fix). Cross-cuts tree/VM/Cranelift since the
parser change affects every backend identically.
Pinned by the engine harness in tests/examples_engines.rs. Three
entries (hit, miss, two-call chain) cover the call-expression value
side and serve as an in-context demo for agents reading the examples
directory before writing nil-coalesce code.
@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 merged commit f375c76 into main May 16, 2026
5 checks passed
@danieljohnmorris danieljohnmorris deleted the fix/qq-call-parse branch May 16, 2026 17:26
danieljohnmorris added a commit that referenced this pull request May 16, 2026
twelve fixes since 0.11.3, surfaced by rerun4 personas plus standing asks: srt-Cranelift TLS desync (#306), CLI auto-run restoration (#307), OP_LISTAPPEND O(n^2) JIT memory regression (#308), precedence-pair hint false-positive on parens (#309), prefix ?? accepts call expression (#310), += pure-shape docs (#311), bare-mutation silent no-op verifier warning ILO-T033 (#312), asin/acos/atan inverse trig builtins (#313), flat cross-engine (#314), cond{~v} discard hint multi-stmt false-positive (#315), rsrt fn xs key-fn overloads (#316), xs.(expr) paren-after-dot diagnostic hint (#317).
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.
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