Accept ?? as a prefix operator#163
Merged
Merged
Conversation
The doc cites this asymmetry across 4+ personas: ?? is the only
binary operator in ilo that doesn't work in prefix form. Every
other binary op accepts both forms (+a b vs a + b, *x y vs x * y),
but ?? required infix only - writing ??c 0 produced the cryptic
"expected expression, got NilCoalesce" error. Agents naturally
reached for the prefix form when composing with calls
(??mget m k 0) and had to fall back to a two-step bind every time.
?? is its own AST variant Expr::NilCoalesce { value, default },
not a BinOp arm, so the existing parse_prefix_binop scaffolding
doesn't apply. Added dedicated arms in parse_expr_inner and
parse_operand that build the same Expr::NilCoalesce shape as the
infix path. The new is_infix_or_suffix_op helper wraps both the
Pratt-table check and a NilCoalesce check - used in the call-arg
termination heuristic without changing Pratt precedence semantics.
The prefix `default` operand parses via parse_expr_inner (same as
the infix path) for full semantic parity: ?? c +a b correctly
treats +a b as the default expression. Earlier draft used the
tighter parse_operand which would have left +b dangling. value
stays as parse_operand since prefix-binary LHSs are operand-shape.
Added NilCoalesce to can_start_operand and to the recursive
prefix-binary scan in scan_prefix_binary_end so the call-arg
loop handles ??c b correctly without bailing.
10 cross-engine cases × 3 engines covering: prefix nil-path, prefix value-path, infix regression (both nil and value), ?? as a call arg (both paths), and the new prefix-default-is-an-expression cases ?? c +a b and the nested ?? c ?? d 0 which exercise full expression depth in the default position. Also fixed a pid-only temp-file race in run_file that surfaced with the new tests writing in parallel - appended an atomic counter to avoid collisions between threads writing the same temp path with different content. examples/prefix-nil-coalesce.ilo demonstrates the new shape via a `dflt o:O n d:n>n;??o d` helper with two run/out cases (nil fallback and value passthrough). One-line header.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
danieljohnmorris
added a commit
that referenced
this pull request
May 11, 2026
check_infix_on_call and check_single_atom_after_op both wrote to fixed-name temp files. Under cargo llvm-cov's parallel test runner, two threads writing different content to the same path raced; whichever finished writing last "won" but the other test's ilo process might have already opened the file with mid-write content, producing ILO-R002 undefined function failures intermittently. Same fix shape as PR #163's run_file: AtomicU64 counter per function, appended to filename along with pid for uniqueness. Relaxed ordering matches the eval_inline.rs precedent (the counter only needs atomic increment; no cross-variable synchronization required).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves a 4+ persona doc citation:
??was the only binary operator in ilo that didn't work in prefix form. Every other binary op accepts both forms (+a bvsa + b,*x yvsx * y), but??required infix only. Writing??c 0produced the crypticexpected expression, got NilCoalesceerror. Agents naturally reached for the prefix form when composing with calls (??mget m k 0) and had to fall back to a two-step bind every time.What's in the diff
parser: accept ?? as a prefix operator—??is its own AST variantExpr::NilCoalesce { value, default }rather than aBinOparm, so the existingparse_prefix_binopscaffolding doesn't apply. Added dedicated arms inparse_expr_innerandparse_operandthat build the sameExpr::NilCoalesceshape as the infix path. Newis_infix_or_suffix_ophelper wraps both the Pratt-table check and a NilCoalesce check, used at the two call-arg termination sites without changing Pratt precedence semantics.The prefix
defaultoperand parses viaparse_expr_inner(matching the infix path) for full semantic parity —?? c +a bcorrectly treats+a bas the default expression. Caught in review: an earlier draft used the tighterparse_operand, which would have left+bdangling.tests + example: pin ?? prefix behaviour across engines— 10 cases × 3 engines covering prefix nil-path, prefix value-path, infix regression,??as a call arg, and the new prefix-default-is-an-expression cases (?? c +a band nested?? c ?? d 0). Also fixed a latent pid-only temp-file race inrun_file(atomic counter appended).examples/prefix-nil-coalesce.ilodemonstrates the new shape via adflt o:O n d:n>n;??o dhelper.Test plan
cargo test --release --features cranelift— full suite greencargo fmt --all -- --checkcleancargo clippy --all-targets --features cranelift -- -D warningsclean?? c 0with c=nil → 0; with c=5 → 5; across all three enginesc??0infix still works (no regression)?? c +a bparses with+a bas a full expression in default position (semantic parity with infix)?? c ?? d 0parses right-associativelyparse_operandvsparse_expr_innerin default position) was caught and fixed before commit