Skip to content

feat(ts): drop the closures std's consumers and entries own where std's do - #438

Merged
moq-bot[bot] merged 5 commits into
mainfrom
quest/m3/ts-drop-order
Nov 2, 2026
Merged

moq-bot[bot] merged 5 commits into
mainfrom
quest/m3/ts-drop-order

Conversation

@kixelated

@kixelated kixelated commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

quest/m3/ts-drop-order.md lists the closures that own a value that drops which rust2-ts still refuses: given to a std consumer that takes another value that drops by value, other than for_each and fold, or called through FnOnce by a mapping that may never call it, as or_insert_with does. A probe of std's consumers found these refused:

  • Iterator::reduce, max_by, min_by, max_by_key, min_by_key, and is_sorted_by_key of an iterator that drops.
  • A map entry's or_insert_with, or_insert_with_key, and and_modify, for std's HashMap and BTreeMap and for hashbrown's maps and HashTable.

Approach

Each mapping now drops the closure where std's does. library::releases replaces iter::releases and covers both groups. called_closures treats a released closure the way it treats a discarded one, so a closure that Fn or FnMut call, given to a FnOnce parameter, becomes rt.once of it and drops after the call.

  • reduce, max_by, min_by: these take owns. std's reduce is self.next()? and then fold. On a non-empty iterator, the closure drops as the fold ends, before the iterator. On an empty one, it drops before the iterator, because parameters drop in reverse order.
  • max_by_key, min_by_key: std maps the iterator by the key's closure, so best takes keyed. On an empty iterator, the closure drops after the iterator, as Map's fields drop. Otherwise it drops as the fold ends.
  • is_sorted_by_key: std's self.map(f).is_sorted() drops f with the Map, after the iterator. rt.iterMap already does that once it is told to drop. A closure that drops now sets drops even when its iterator drops nothing. A closure lent through a reference, boxed or not, is rt.lent; one it owns drops with the map, as std's does.
  • Entries: rt.entryOrInsertWith, entryAndModify, tableOrInsertWith, tableAndModify, and their Mut forms take what drops the closure. They call it when they don't call the closure: on an occupied entry for or_insert_with, and on a vacant one for and_modify.

Tests:

  • A new task-fixture step, released (ts/fixtures/task/src/drops/released.rs, 7 scenarios), runs differentially against native wasm32. It covers each consumer over:
    • a take that holds leftovers;
    • an empty iterator;
    • an iterator that drops nothing;
    • a lent closure;
    • each entry method on both maps, occupied and vacant, by a closure that moves its guard out and by one that Fn calls;
    • a key closure that panics mid-fold.
  • The stock fixture's HashTable steps cover hashbrown's or_insert_with and and_modify by a closure owning a drop counter.

Mutation checks:

Mutation Differences
Drop the keyed closure before an empty iterator 912 / 35,424
Drop max_by's comparator after an empty iterator 810
Swap reduce's empty-iterator order 1,838
Release best's closure after the fold, not inside it 1,672
Skip the occupied or_insert_with drop 1,188
Skip the vacant and_modify drop 1,188
is_sorted_by_key ignoring a closure that drops 1,584
rt.once not dropping after the call 3,770 task; 97 stock
tableOrInsertWith not dropping 15 stock
tableAndModifyMut not dropping 296 stock

Each now gives 0.

The reject fixture's called_closure is deleted. It translates now, and the released step covers it.

Impact

  • Translator output: the closures above translate and drop where std's do, where they were refused.
  • Runtime, breaking: rt.iterReduce, rt.iterMaxBy, rt.iterMinBy, rt.iterMaxByKey, and rt.iterMinByKey take owns before the rt.None they take last. Recorded in ts/MIGRATE.md.
  • Runtime, additive: optional trailing dropF on rt.entryOrInsertWith, entryOrInsertWithMut, entryAndModify, entryAndModifyMut, tableOrInsertWith, tableAndModify, and tableAndModifyMut.
  • No benchmark: the paths without drops gain one undefined check per call, not per item.

Alternatives

  • Listing each entry's closure in discards by its type-argument index. The index differs between HashMap's Entry (K, V, F) and BTreeMap's (K, V, A, F), so a by-name releases with the closure taken as the last type argument is simpler.
  • Wrapping the whole call in rt.dropAfter, as a spent closure is. That drops the closure after the iterator, where std drops it before, so it gives the wrong order on every non-empty fold.

Follow-ups

  • quest/m3/ts-drop-order.md now holds only the zip, flatten, and B-tree collect refusals. Its plan named partition, but partition is refused as an unsupported call whatever the closure holds.
  • New quest/m3/ts-closure-calls.md: rfold, try_fold, try_for_each, partition, repeat_with, scan, sort_by_cached_key, LazyCell::new, Cell::update, and RefCell::replace_with have no mapping at all.
  • New quest/m3/ts-string-closures.md: String::retain and str searches by a closure that drops. These use a separate refusal, outside called_closures.

Choices made without asking

  • Covered hashbrown's map and table entries too. They share the owner name Entry, so leaving them out would have meant a type check just to keep them refused.
  • Named the new step released, after the runtime's release.
  • Left String::retain and str patterns to a quest. They are a different refusal path.

Review

Local quest review: Codex, gpt-6.1-sol, medium effort. Round 1 on head 9d4e6d8ca53ebca4f2dbae95d2227044172b643c: no findings.

Head 80168791 adds only quest text: ts-closure-calls drops rfold and links #436's ts-iter-back-folds for folds from the back, and ts-drop-order notes the B-tree collect follows the sort port. Review skipped for that commit; just ci passes on it.

Head bde31876 merges origin/main: ts/MIGRATE.md keeps both sides' entries, and the reject and stock snapshots are regenerated (the census drops both Hasher::write's refusal from main and Entry::or_insert_with's from this PR, 229 refusals). Review skipped for that merge; just ci passes on it.

Checks: just ci passes locally on 9d4e6d8c. One self-caught bug along the way: is_sorted_by_key briefly dropped a closure lent as Box<&F>. The lang fixture's differential test found it (35 differences), and it is fixed by reading the lend through called_through.

(written by Claude Opus 5.5)

🤖 Generated with Claude Code

kixelated and others added 2 commits November 2, 2026 04:06
Quest-Claim: a234456d49680711
…'s do

reduce, max_by, min_by, their _by_key forms, is_sorted_by_key, and a map
entry's or_insert_with, or_insert_with_key, and and_modify translate a
closure that owns a value that drops, dropping it where std's does,
proven by the task fixture's new `released` step against native wasm32.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title Drop order in std's folds and iterators feat(ts): drop the closures std's consumers and entries own where std's do Nov 2, 2026
@kixelated
kixelated marked this pull request as ready for review November 2, 2026 13:27
@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Important

  • Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

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: 20c3bb4a-aca2-4c00-bda3-7c238d132797
  • 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

…t to the sort port

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated enabled auto-merge (squash) November 2, 2026 14:36
@moq-bot

moq-bot Bot commented Nov 2, 2026

Copy link
Copy Markdown

quest land: this pull request conflicts with a pull request queued ahead of it, in these files:

  • #424: ts/MIGRATE.md, ts/fixtures/reject/census.md, ts/fixtures/reject/expected.txt, ts/fixtures/stock/expected/maps.ts

Once one of them lands, this one conflicts with the base and is ejected when its turn comes. Resolve the overlap before then, such as by coordinating with that author, or by merging the base once it lands.

@moq-bot
moq-bot Bot disabled auto-merge November 2, 2026 16:01
@moq-bot

moq-bot Bot commented Nov 2, 2026

Copy link
Copy Markdown

quest land removed this pull request from the queue: PR #438 merge push failed: remote: Permission to moq-dev/rust2.git denied to moq-bot[bot].
fatal: unable to access 'https://github.com/moq-dev/rust2.git/': The requested URL returned error: 403; the rest of the batch stays queued.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated enabled auto-merge (squash) November 2, 2026 17:35
@moq-bot
moq-bot Bot merged commit c50edc8 into main Nov 2, 2026
@moq-bot
moq-bot Bot deleted the quest/m3/ts-drop-order branch November 2, 2026 18:13
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