Skip to content

feat(ts): translate a B-tree's extract_if, with its Debug - #557

Merged
kixelated merged 6 commits into
mainfrom
quest/m3/ts-btree-extract-if
Nov 4, 2026
Merged

kixelated merged 6 commits into
mainfrom
quest/m3/ts-btree-extract-if

Conversation

@kixelated

@kixelated kixelated commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

rust2-ts refused BTreeMap::extract_if and BTreeSet::extract_if (std 1.91, with a range). #547 landed the hash map's and set's; this finishes the B-tree's, part of the m3 "all of std" plan. No consumer calls them yet.

Approach

  • rt.BTreeExtractIf keeps std's cursor: the leaf and index of the entry it reads next. A removal that rebalances only merges the leaf with one neighbor or takes from it. So the cursor follows without comparing keys: it moves into the previous leaf if this one merged into it, or shifts by what that leaf gave to this one's front.
  • Matches std's ExtractIfInner:
    • It reads start_bound once, only for a map with a root (m.r).
    • It reads end_bound at each key, before the predicate, and compares by PartialOrd::le/lt. A key of the crate's own passes its own tests, as range passes eq/gt.
    • It never panics on a backwards range or one excluded at both ends.
    • It stops for good past the range's end, at the tree's end, or on a panic in the predicate or the bound, since the cursor is cleared before each call.
    • size_hint is (0, Some(len)) of the live map.
  • The map's &mut V predicate gets a place where JS copies the value (place_call), so writes land on kept entries too.
  • Debug writes ExtractIf { peek: .., .. }: the next entry, or a set's key, even past the range's end until it stops there, and None once stopped.
  • Drop: the closure drops, then the range, as std's field order does. A range that drops is handed to the iterator through the new Ranged::held. rt.boundsOf now returns { startBound, endBound } so each bound can be read on its own. Before, it was one function that read both.
  • Tests:
    • collections::btree_extracts (differential): maps and sets of up to 510 keys, every range form, partial reads, Debug ({:?} and {:#?}), and maps with no root.
    • Keys and values that log their drops and their le/lt, plus a closure and range that drop with the iterator.
    • A RangeBounds of the crate's own that logs each bound read.
    • tests/btree.ts against std (runtime.rs): extract_if over random ranges of a ~20k-entry tree, read part way, with the tree's shape checked after.
    • Mutation-checked: dropping either half of the cursor's move after a rebalance, ignoring le, swapping the drop order, reading the start of a rootless map, and peeking after a stop each fail the differential, and the first also fails btree.ts.

Impact

  • New: rt.btreeExtractIf, rt.btreeExtractIfMut, rt.btreeExtractKeysIf, rt.fmtBTreeExtractIf.
  • Breaking: rt.boundsOf returns a { startBound, endBound } record instead of a function (ts/MIGRATE.md). Generated code is unchanged.

Alternatives

  • Flatten and rebuild, as retain does: O(n) per next, and wrong for a read-part-way iterator unless it rebuilds on drop. Rejected.
  • Defer rebalancing until the iterator drops: leaves a malformed tree if it is forgotten (mem::forget). Rejected.
  • Keep rt.boundsOf a function and call it at every key: that would read start_bound again at each key, which std doesn't do.

Follow-ups

  • removeAt, which pop_first, pop_last, remove, and now extract_if share, still calls Ord::cmp to rebalance where std's removals call none. A B-tree's searches call Ord as std's do now names these removals in its Goal (5ca5a9b).
  • The fixtures' nightly std (1.101) drops a whole multi-level B-tree in postorder (drop_tree), while the runtime drops in key order (stable 1.98.1's order). This was already the maintainer's decision of 2026-10-30, recorded in ts/README.md, so the new fixture drains its logged map in order. No new quest.

Review

quest review: Codex, gpt-6.1-sol, medium effort. Round 3, head d724cc2: no findings.

  • Round 1: removal calls Ord::cmp, unlike std's. Refuted as the existing removeAt behavior (pop_first has it too), which the B-tree search-order quest now covers.
  • Round 2: the cursor's re-find by key added calls of its own. Fixed in 7504ae8: the cursor follows a rebalance structurally, with no comparisons.

🤖 Generated with Claude Code

kixelated and others added 6 commits November 3, 2026 15:41
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t comparing keys

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title A B-tree's extract_if feat(ts): translate a B-tree's extract_if, with its Debug Nov 4, 2026
@kixelated
kixelated marked this pull request as ready for review November 4, 2026 00:13
@pullfrog

pullfrog Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

This run was cancelled before it finished.

Pullfrog  | View workflow run | via Pullfrog

@kixelated
kixelated added this pull request to the merge queue Nov 4, 2026
@coderabbitai

coderabbitai Bot commented Nov 4, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 33 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e0dce2ee-221d-42f5-8af9-e6d96309b9cf
Commits

Reviewing files that changed from the base of the PR and between 16de89b and d724cc2.

Files selected for processing: 18
  • quest/m3/README.md
  • quest/m3/ts-btree-extract-if.md
  • quest/m3/ts-btree-search-order.md
  • ts/MIGRATE.md
  • ts/README.md
  • ts/fixtures/lang/collections/expected/collections.Logged.ts
  • ts/fixtures/lang/collections/expected/collections.Window.ts
  • ts/fixtures/lang/collections/expected/collections.ts
  • ts/fixtures/lang/collections/expected/harness.ts
  • ts/fixtures/lang/collections/expected/lib.ts
  • ts/fixtures/lang/collections/src/collections.rs
  • ts/fixtures/lang/collections/src/lib.rs
  • ts/runtime/rt/collections.ts
  • ts/src/lower/collections.rs
  • ts/src/lower/fmt.rs
  • ts/src/lower/iter.rs
  • ts/tests/btree.ts
  • ts/tests/runtime.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Merged via the queue into main with commit 593c502 Nov 4, 2026
7 of 8 checks passed
@kixelated
kixelated deleted the quest/m3/ts-btree-extract-if branch November 4, 2026 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant