docs: Phase 2 closure capture works on every engine (PR5 of 5)#394
Merged
Conversation
The tree-only caveats in SPEC's Inline lambdas section and the Default engine section pre-date #384/#385/#386/#387/#389/#391. Closure capture now runs natively on tree, VM, and Cranelift JIT/AOT with no per-engine fallback. Flip the language in SPEC, then let build.rs regenerate ai.txt and the SKILL.md mirror block. The standalone gotcha line in SKILL.md picks up the same flip.
After phase 2 landed across the VM and Cranelift backends, the only remaining unsupported case for OP_MAKE_CLOSURE is a single inline lambda capturing more than 255 free variables (the 8-bit cap of the instruction's count field). Update the error message and the variant's doc comment to reflect that, and refresh the stray-Value::Closure comment in NanVal conversion so it doesn't claim the VM bails on captures.
ILO-P017 was originally the "inline lambda captures outer scope" parser error. Phase 2 closure capture absorbed that code path, and the code is now used exclusively for use-import resolution failures. Rewrite the registry long-form to match its current behaviour, with a note explaining where the closure-capture wording went. Also fix the stale doc comment on parse_inline_lambda that still mentioned the Phase 1 rejection path.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…c-cleanup # Conflicts: # SPEC.md # ai.txt # skills/ilo/SKILL.md
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
Final doc cleanup for the Phase 2 closure-capture rollout. After #384 (VM closures), #385 (Cranelift closures), #386 (window-view JIT), #387 (partition off bridge), #389 (mapr off bridge), and #391 (srt/grp/uniqby off bridge), Phase 2 closure capture runs natively on every engine with no per-engine fallback. This PR flips every "tree-only" / "Phase 2 is tree-only" claim across the doc surface and tightens a couple of stale error-text and code-comment leftovers.
Lands at the same time as #391; merge order doesn't matter, the doc claim becomes true either way once both are in main.
What's in the diff
ai.txtandskills/ilo/SKILL.mdare regenerated from SPEC viabuild.rs. SKILL.md's standalone gotcha line picks up the same flip.OP_MAKE_CLOSUREgenuinely caps at 255), but the message and doc comment used to say "tree-only" which is no longer true. Stray-Value::Closurecomment inNanVal::from_valueupdated the same way.use-import resolution. Rewritten to match. Stale parse_inline_lambda doc comment also fixed.Test plan
cargo build --release --features craneliftregeneratesai.txt+skills/ilo/SKILL.mdfrom the SPEC editscargo test --release --features cranelift- 6864 passed, 0 failedcargo fmt --checkcleancargo clippy --release --features cranelift --all-targets -- -D warningscleanPhase 2/tree-only/closure capture- no user-facing claims to flip thereFollow-ups
src/main.rsstill point at the repurposed code; the parser path that used to raise ILO-P017 for closure captures is gone (Phase 2 handles those viaMakeClosureinstead). Out of scope for a doc PR; the runtime behaviour is correct and the registry now reflects what users actually see.