feat: soft-deprecate tree-walker (drop --run-tree CLI, internalise interpreter)#439
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The tree-walking interpreter has been the slowest engine since the VM became the default, and the --run-tree flag was mostly a foot-gun: agents reached for it for debuggability and paid an 8x wall-clock tax on real workloads (see the 2026-05-12 routing-tsp / opt4.ilo notes). This soft-deprecates the engine. --run-tree and its --run alias are removed from the public CLI: they used to compile to Engine::Tree via clap, now they fall through to the unknown-flag guard with a clean error pointing at --run-vm and --jit. The tree-walker itself stays in-tree as the dispatch target for the HOFs / regex / fmt / IO shapes the VM and Cranelift haven't lifted natively yet (see the 2026-05-18 STOPPED entry); the VM bails to it transparently. interpreter is gated behind #[doc(hidden)] in lib.rs so it disappears from rustdoc / IDE completion, but stays pub for the in-tree bin and the cross-engine tests. The run_tree / run fields on RunArgs are kept with #[arg(skip)] so internal construction sites (REPL, dispatch, tests asserting effective_engine) still compile. Real removal is deferred to 0.13.0+ once the bridge consumers are lifted natively and the shared runtime types are extracted to a non-engine module. The wr (3-arg dynamic-format) VM compile error used to suggest --run-tree as the workaround; that hint is now stale, so the message just states the gap.
…removal cli_run_flag_placement got a full rewrite: the per-position --run-tree tests now exercise --run-vm, the two conflicting-flags tests pin the --run-vm vs --jit pair, the --run-alias tests assert clean rejection via the unknown-flag guard, and the file-path positional tests track the same flag. cli_integration.run_tree_basic_execution was renamed to run_vm_basic_execution_via_subcommand and repointed at --run-vm so the clap subcommand path still gets coverage. regression_unknown_flag_guard gains a positive test that --run-tree now exits non-zero with the flag named in stderr. eval_inline had a handful of legacy --run alias invocations (the tree-walker shorthand); those flip to --run-vm.
Bulk-replace --run-tree with --run-vm in every cross-engine regression test. The tree-walker is no longer publicly selectable, but the VM bails to it transparently for the bridge ops (regex, fmt, ctx-arg HOFs, io shapes), so coverage of tree-walker code paths is preserved through VM invocations. ENGINES_ALL arrays shrink from [tree, vm, jit] / [tree, vm] to [vm, jit] / [vm]; single-element arrays trip clippy's single_element_loop where they used to be multi-engine loops, so the affected files gain a file-level #[allow] with a comment pointing at this change. coverage_interpreter widens its error-code asserts from a tight ILO-R009 (interpreter error code) to a union of ILO-R003 / R004 / R009 / R012 because the VM and Cranelift use different codes for the same shape errors. wr_unknown_format_errors now accepts the VM's 'undefined function' compile-time refusal since the VM has no native 3-arg wr-with-dynamic-format path and --run-tree is gone. examples_engines drops the duplicate tree row from its engine list. engine-matrix/run-matrix.sh + README drop the tree column from the audit matrix.
Tests added in 0.12.0+ (regression_aot_strconst_interning, regression_run_builtin, regression_ls_freed_as_binding, regression_field_access_underscore_typed, regression_fs_builtins, regression_jpth_object_array_typed, regression_cond_body_in_loop, regression_listlit_builtin_call_hint) still had --run-tree in their ENGINES arrays or per-test invocations. Sweep them to --run-vm (and drop the duplicate tree-named tests where the vm/jit pair already covers the shape). Update stale --run-tree mentions in assert messages on the *_accumulator_tree files. The example .ilo comment headers reference --run-tree in pre-fix narratives, update them so agents reading the examples for syntax see the current public surface. Also pick up trailing-newline fmt drift in 11 regression files flagged by cargo fmt after the sweep.
When 0.13.0 hard-drops the tree-walker, a maintainer touching the VM's `is_tree_bridge_eligible` table needs an unambiguous list of what user code expects from the bridge today. The existing `regression_builtin_bridge` covers rgx / rgxall / fmt / rd / rdb; this file fills in the gaps that landed across PR3, PR3b, PR3c: - rgxall1 (flat first-capture convenience) - rgxsub (regex substitute, with $-backrefs) - fmt2 (decimal precision) - sleep (lossless Nil round-trip) - ct 2-arg and 3-arg ctx (count by predicate) - rsrt 2-arg and 3-arg ctx (descending sort by key) - map / flt / srt 3-arg ctx (closure-bind variants) - fld 4-arg ctx (closure-bind accumulator) - composition (chained bridge ops in one program) Every test runs on --run-vm and --jit (when cranelift is on). The tree-walker is exercised transitively through the VM bridge path on the --run-vm invocation, so cross-engine parity here = cross-engine parity through the bridge. Plus `examples/tree-bridge-invariants.ilo` so an agent reading the examples sees one canonical shape per bridge family; the engine harness in tests/examples_engines.rs replays it across VM/JIT/AOT.
… removal The original PR #423 doc-sync commit hit semantic conflicts after main's skill docs were restructured (the engines page rewritten, ilo-edit-loop and ilo-errors added). Redo the doc edits against current main: - SPEC.md: drop --run-tree from the in-process runner list, retire the canonical-semantics-reference sentence (every public backend handles Phase 2 captures natively), and add a new "Tree-walker is internal-only" paragraph naming every entry in is_tree_bridge_eligible plus the two bridge-coverage test files. - ai.txt: same three edits, single-line. - README.md: drop the "ilo Interpreter" row from the benchmarks table. The VM and JIT rows are what agents actually choose between, and keeping the interpreter row implied --run-tree was a supported choice. - skills/ilo/ilo-engines.md: full rewrite. Drops the Tree-walk row and the `--run` alias note, lists three public backends, calls out the internal-only tree-walker with the bridge ops below the table, and trims the benchmarking line to vm/jit. Captures-run- natively guidance loses the tree caveat. - skills/ilo/ilo-edit-loop.md: R012 guidance loses the "or --run-tree" workaround (every public backend supports Phase 2 captures). - skills/ilo/ilo-errors.md: the `--engine tree` mystery-arity pattern points at --run-vm / --jit instead of --run-tree. - CHANGELOG.md: move the --run-tree removal line from the 0.12.0 Breaking section (which never shipped that change) to 0.12.1 where it actually lands, with the longer framing that calls out the bridge consumers.
The rebase onto main pulled in ai.txt regen from build.rs, and the expanded tree-bridge bullet I added to skills/ilo/ilo-engines.md pushed the aggregate skill budget to 5049 (over the 5000 CI gate). Tightened the tree-walker-is-internal-only paragraph, condensed the intro and AOT footer, and re-ran the build to refresh ai.txt from the rebased SPEC. Aggregate now sits at 4999.
danieljohnmorris
force-pushed
the
feature/soft-deprecate-tree-walker-v2
branch
from
fbd0474 to
495375b
Compare
#424 landed regression_aot_closures.rs after this branch's first sweep, so its in-proc engine loop included the now-removed flag. VM and JIT cover the same matrix; AOT remains its own arm.
…val_inline strings #435 added regression_csv_multiline_roundtrip.rs with an engines list that still included --run-tree, which the soft-deprecate removes. VM round-trip alone covers the bug fix scope. Also refreshed an error-message string in eval_inline.rs that still referenced --run-tree after the prior sweep renamed the actual flag to --run-vm.
#437 landed regression_rand_alias.rs after this branch's prior sweep, so its in-proc engine loop and the alias-hint test still referenced the removed flag. VM (and JIT under the cranelift feature) cover the same coverage matrix. Also refreshed stale --run-tree comment in eval_inline.rs's run_interp_runtime_error.
…miscompile note Merge from main re-introduced ~30 tokens that earlier trim pass had removed. Also dropped the line claiming AOT miscompiles HOFs taking fn-values, since PR #424 (AOT closures program-embed) fixed that. Aggregate now 4982 of 5000.
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
Removes
--run-treeand--runfrom the public clap surface so users can't select the tree-walker engine directly. The tree-walker module is internalised behindpub(crate)and#[doc(hidden)], butEngine::Treestays as the in-VM bridge for HOF dispatch (regex / fmt / rd-family / 3-arg-ctx variants of map/flt/fld/srt/rsrt). 161+ test files swept from--run-treeto--run-vmsince the VM bridges to the tree-walker transparently. This is plumbing for the 0.13.0 hard-drop of the tree-walker (per the [[project-ilo-tree-walker-drop-013]] memory note, blocked on PR3d / PR3e / runtime extraction).Fresh redo of the closed #423 since the original branch hit 135 lines of doc-merge conflicts after main's skill docs reshuffled.
What's in the diff
cli: drop --run-tree / --run from the public engine surface— removes the flags from clap; unknown-flag guard catches them with a clean errortests: rewrite cli flag-placement / unknown-guard for the --run-tree removal— pin the post-removal CLI shapetests: sweep --run-tree to --run-vm across the regression suite— 161 files updatedtests: sweep newer --run-tree refs that landed since the original sweep— catches references that landed on main during the rebasetests: pin the tree-bridge dispatch contract for 0.13.0 maintainers— cross-engine invariant tests pinning HOF / regex / fmt-family / rd-family bridge behaviour, so a future PR touching tree-walker logic without realising it can't silently break user codedocs: sync SPEC / ai.txt / skills / changelog / README for --run-tree removal— written against current main's reshuffled skill docsTest plan
cargo test --release --features craneliftpasses the full suitecargo fmt && cargo clippy --release --features cranelift -- -D warningscleanilo --run-tree foo.iloproduces a clean unknown-flag error, not a misleading hintilo --run-vm foo.ilostill exercises tree-walker for bridge ops (regex, fmt-family, rd-family) via the VM transparentlyTree-bridge invariants observed
While editing for the rebase + sweep I confirmed which paths the tree-walker still owns end-to-end (and which the 0.13.0 hard-drop work needs to migrate before this code can be deleted):
rgx,rgxall,rgxall1,rgxsub) — VM falls back to tree for all of thesefmt,fmt2with complex args)rd,rdb,rdjl)map/flt/fld/srt/rsrt(closure-bind dispatch)sleep,ct,rsrtThese are the surface area for PR3d (migrate to native VM ops) and PR3e (extend OP_CALL_DYN for 3-arg ctx) before the tree-walker can be deleted in 0.13.0.
Follow-ups
src/interpreter/intosrc/runtime/src/interpreter/entirely (the hard drop)