You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(ts): collect into and iterate a crate's own collections through std's adaptors - #592
std's iterator adaptors refused a crate's own collection: collect into a type with its own FromIterator, and flat_map/flatten over a type with its own IntoIterator. moq-net's model reaches these through moq-pattern's Patterns (auth.rs, model/origin.rs, Patterns::rebase), rangemap's RangeInclusiveSet (model/subscription/runs.rs), and smallvec's SmallVec (kio's log.rs).
Approach
rs.Iter mappings already exist for every adaptor; what was missing is std's generic code calling the crate's impl.
collect into the crate's own collection calls its from_iter, instantiated for the receiver's type, with the receiver itself, as std's collect does. A collect into a Result or Option of one hands it the GenericShunt std's try_process does, which is an rs.Iter.
into_iter of a crate's own collection calls its into_iter and iterates what that returns. That one place covers every adaptor taking an IntoIterator: flat_map, flatten, chain, zip, Vec::extend, and FromIterator of std's collections.
The impl is called through fn_value, so a binding or #[track_caller] impl is handled as anywhere else.
The runtime's Result/Option shunts now read on past an Err or None, as std's GenericShunt::next does, and a later Err replaces the earlier one, which then drops. Before this PR only std's collections read a shunt, and they stop at the first None. A crate's from_iter can read again. That is why iterCollectResult now takes the error's dropper, and the task snapshot shows the extra argument.
New fixture lang/collects runs against native Rust (505 cases, 0 differences). Its items log as they are made, read, and dropped, so the log shows laziness and drop order. It covers: collect that stops early, leaving the rest of its source to drop; Result and Option collects, including a collection that reads past the residual; a generic collection; a crate iterator by value and by by_ref; flat_map read part way and then next_back; flatten by value and by reference; flat_map into a type whose iterator is the crate's own and drops; and chain, zip, and extend.
I mutation-tested it by editing the translated output and runtime by hand. Each mutation failed the run:
Mutation
Differences
No drop of the crate's iterator in flat_map
9
No drop of a Result's collection after an Err
1
An eager collect source
35
A reversed &Bag iterator
5
The old shunt stopping after Err
17
No drop of a replaced Err
17
The old shunt stopping after None
20
Census of ts/moq/model.toml at the current moq pin (ffcb7cd, bumped by #390): origin/main's driver counts 93 refusals and this branch counts 82, with exactly the 11 collect/flat_map/flatten refusals gone and no new ones. moq-net as a whole goes from 434 to 409. The epic now says 82; its old 69 was counted at the previous pin. Programs that don't use this translate as before: every other snapshot is unchanged but task's extra argument, and just ts moq passes.
Impact
No public API change. The translator's output changes only where a program collects into, or iterates through std's adaptors, a crate's own FromIterator/IntoIterator type, which translation used to refuse. rs.iterCollectResult takes an optional error dropper.
Alternatives
Lowering std's Iterator::collect/FlatMap MIR instead of the rs.Iter mappings: every other adaptor is mapped, and std's bodies here go through specialization and GenericShunt internals.
Follow-ups
The moq pin bump in feat(ffi): export kio's clock as moq_clock_* #390 added model refusals that no quest owns yet: util::idle::Idle's Option<K> field as Option<()> (16), arrayvec's ArrayString (const generics, begin_panic, str::as_ptr), Weak::as_ptr, thread::yield_now, and Box::new_uninit_slice. The epic's breakdown of what remains predates the bump. Worth a /quest-plan pass.
Review
mechanical fix by codex / Codex: Merged base and regenerated six stale collects snapshots; passed in ts/ on pinned nightly: UPDATE_EXPECT=1 cargo test --locked --test fixtures lang::collects -- --exact; cargo build --locked; cargo test --locked; tsc -p runtime. (head ae2e09f)
Codex (quest review), gpt-6.1-sol, medium effort:
Round 1 (fb87783), medium "Preserve GenericShunt reads after an error": fixed in 8f3600e. It is reproduced by the Tries collection in lang/collects and verified by mutation.
Round 1 of a new session after AGENTS.md changed (0e70bdc): no findings.
Round 2 (89f73a2), after merging origin/main: no findings.
…std's adaptors
std's `collect` into the crate's own `FromIterator` type, and into a `Result`
or `Option` of one, calls its `from_iter`; `flat_map`, `flatten`, `chain`,
`zip`, `extend`, and the rest that take an `IntoIterator` iterate the crate's
own collection by its `into_iter`. Drops moq-net model's census from 78 to 71.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
Iterators into a crate's own collections
feat(ts): collect into and iterate a crate's own collections through std's adaptors
Nov 4, 2026
…'s does
std's `GenericShunt::next` reads on after a residual and replaces it,
dropping the old one. The runtime's shunts stopped for good at the first,
which a crate's own `FromIterator` that reads again can observe.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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: 4d384aa8-dc90-4dd8-a9b8-5b4f9bad6387
No actionable comments were generated in the recent review.
Walkthrough
The compiler now resolves crate-owned FromIterator and IntoIterator implementations in iterator lowering. Result collection replaces earlier errors with later errors and can drop replaced errors. Option collection continues reading after None. A new language fixture tests collection, flattening, chaining, and zipping with crate-defined types.
Merge Risk: Minimal · previous review 89f73
No merge-blocking issue was identified in the iterator collection changes. Mergeable subject to normal checks.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 41.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Title check
The title names the main change: letting std's collect and iterator adaptors use a crate's own FromIterator and IntoIterator implementations. The raw summary confirms this, with changes in ts/src/lowe…
Description check
The description is related to the changeset. It explains the problem (std adaptors refusing a crate's own collections), the approach (calling the crate's from_iter and into_iter, and changing the Resu…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title names the main change: letting std's collect and iterator adaptors use a crate's own FromIterator and IntoIterator implementations. The raw summary confirms this, with changes in ts/src/lower/iter.rs, the runtime shunts, and the new lang/collects fixture. The title is specific and follows the conventional-commit format.
Full details: Description check
Explanation
The description is related to the changeset. It explains the problem (std adaptors refusing a crate's own collections), the approach (calling the crate's from_iter and into_iter, and changing the Result/Option shunts), the new lang/collects fixture, mutation testing, census results, and the iterCollectResult dropper argument. These match the raw summary, which lists the same files and API change.
Full details: Docstring Coverage
Explanation
Docstring coverage is 41.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 34 functions across 17 files. (4 skipped: 4 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Simplify code
Commit to this branch
Create a new PR
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.
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
quest land watch: the merge queue removed this pull request at 89f73a2 because its merge_group build failed (failed_checks).
The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.
Queue / Queue build / Build (job) failure at step Check and test the merge group:
…guard(21)));
430 + let _ = counts.entry(counted(20)).or_insert(guard(21));
|
warning: calls to `std::mem::drop` with a reference instead of an owned value does nothing
--> fixtures/task/src/drops.rs:431:2
|
431 | drop(counts.entry(counted(22)).or_insert_with(|| guard(22)));
| ^^^^^------------------------------------------------------^
| |
| argument has type `&mut Guard`
|
help: use `let _ = ...` to ignore the expression or result
|
431 - drop(counts.entry(counted(22)).or_insert_with(|| guard(22)));
431 + let _ = counts.entry(counted(22)).or_insert_with(|| guard(22));
|
warning: calls to `std::mem::drop` with a reference instead of an owned value does nothing
--> fixtures/task/src/drops.rs:432:2
|
432 | drop(counts.entry(counted(20)).and_modify(|_| ()).or_insert(guard(23)));
| ^^^^^-----------------------------------------------------------------^
| |
| argument has type `&mut Guard`
|
help: use `let _ = ...` to ignore the expression or result
|
432 - drop(counts.entry(counted(20)).and_modify(|_| ()).or_insert(guard(23)));
432 + let _ = counts.entry(counted(20)).and_modify(|_| ()).or_insert(guard(23));
|
warning: for loop over an `Option`. This is more readably written as an `if let` statement
--> fixtures/task/src/drops/iters.rs:22:11
|
22 | for g in Some(guard(7)) {
| ^^^^^^^^^^^^^^
|
= note: `#[warn(for_loops_over_fallibles)]` on by default
help: to check pattern in a loop use `while let`
|
22 - for g in Some(guard(7)) {
22 + while let Some(g) = Some(guard(7)) {
|
help: consider using `if let` to clear intent
|
22 - for g in Some(guard(7)) {
22 + if let Some(g) = Some(guard(7)) {
|
warning: `task-fixture` (lib) generated 12 warnings (run `cargo fix --lib -p task-fixture` to apply 1 suggestion)
Finished `differential` profile [optimized + debuginfo] target(s) in 14.33s
Compiling share-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/share)
Finished `differential` profile [optimized + debuginfo] target(s) in 13.99s
test stock ... ok
Compiling share-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/share)
Finished `differential` profile [optimized + debuginfo] target(s) in 0.26s
Compiling share-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/share)
Finished `differential` profile [optimized + debuginfo] target(s) in 0.25s
test share ... ok
test task ... ok
failures:
---- lang::collects stdout ----
thread 'lang::collects' (3006420) panicked at tests/fixtures.rs:135:9:
lang/collects: collects.Bag.ts differs from its snapshot; rerun with UPDATE_EXPECT=1
- const args2: rs.Fmt[] = [(f: rs.Formatter) => rs.usizeDisplay(part, f)];
- const _26: string = rs.fmtFormat(["bag of ", args2[0]]);
- _25.value.items.push(_26);
- rs.refMutRelease(_25);
- }
- return new Bag(items);
- }
-
+ const _26: string = rs.fmtFormat(["bag of ", (f: rs.Formatter) => rs.usizeDisplay(part, f)]);
+ _25.value.items.push(_26);
+ rs.refMutRelease(_25);
+ }
+ return new Bag(items);
+ }
+
+ export function fromIter$iter_FromIterator_collects_Item_iter_Map_iter_Rev_slice_Iter_u8_collects_chains_closure2(iter: rs.Iter<Item.Item>): Bag {
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
lang::collects
test result: FAILED. 87 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 333.27s
error: test failed, to rerun pass `--test fixtures`
error: recipe `test` failed on line 39 with exit code 101
error: recipe `ci` failed on line 28 with exit code 101
##[error]Process completed with exit code 101.
Post job cleanup.
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git version
git version 2.55.0
Temporarily overriding HOME='/var/cache/github-runner-work/_temp/f840b248-666e-4858-85ce-d6ed96331ae3' before making global git config changes
Adding repository directory to the temporary git global config as a safe directory
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --global --add safe.directory /var/cache/github-runner-work/rust2/rust2
Removing SSH command configuration
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp core\.sshCommand
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
Removing HTTP extra header
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
Removing includeIf entries pointing to credentials config files
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir:
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive git config --local --show-origin --name-only --get-regexp remote.origin.url
A job completed hook has been configured by the self-hosted runner administrator
##[group]Run '/nix/store/gx2v6q84b0w43bafm0whkdx93qyxbpps-ci-cache-gc.sh/bin/ci-cache-gc.sh'
shell: /nix/store/1mv3qz005gkbalxfghk4y3s8ayv4d2dl-bash-interactive-5.3p15/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
evicted 18 objects and 0 action results (20.3 MiB logical); 50.0 GiB logical remain
dropped 1 stale checkout records
dropped 1 session event streams
Cleaning up orphan processes
Queue / Queue (job) failure at step Require the build to pass:
The new lang::collects fixture snapshots were stale against the current base. Merged base cleanly and regenerated six snapshot files using the pinned nightly; no source behavior changed. Validation passed: cargo build --locked, cargo test --locked (8 unit, 88 fixture, and 10 runtime tests), and tsc -p runtime, all from ts/.
quest land watch: armed auto-merge on this pull request at ae2e09f, so it joins the back of the merge queue once its required checks pass, with the land fixer's fix (codex / Codex): Merged base and regenerated six stale collects snapshots; passed in ts/ on pinned nightly: UPDATE_EXPECT=1 cargo test --locked --test fixtures lang::collects -- --exact; cargo build --locked; cargo test --locked; tsc -p runtime..
The reason will be displayed to describe this comment to others. Learn more.
✅ No new issues found.
Reviewed changes — this run reviewed commit range 8cd50cef..89f73a2d (23 files, +1486 -28): the translator stops refusing collect/flat_map/flatten through a crate's own FromIterator/IntoIterator, and makes the runtime's Result/Option shunts read on like std's GenericShunt.
New own_impl resolution gate (ts/src/lower/iter.rs:520): resolves std's FromIterator/IntoIterator diagnostic fns via Instance::try_resolve, refuses over has_param() || has_aliases() (an established idiom in drop.rs/glue.rs), and requires b.t.own of the resolved impl's def_id, called through b.fn_value. The three call sites wire it into collect (raw receiver passed, as std's collect passes self), MIR FromIterator::from_iter, and peeled_into_iter, and instance args [Self, A, T] are in the right order at each.
Shunt parity with std (ts/runtime/rs/iter.ts): ResultShunt/OptionShunt drop their after-error latch, so a consumer reading again reads on, and a later error replaces the earlier one (dropped by the new dropError parameter of iterCollectResult). This matches upstream std exactly, verified against library/core/src/iter/adapters/mod.rs: GenericShunt stores residual: &'a mut Option<R> so replacing drops the earlier residual, has no latch, and reports size_hint(0, Some(0)) once a residual is seen.
Differential fixturelang/collects covers the flat_map drop of a crate iterator, collect that stops early and drops its residual source, Result/Option collects including one whose collection reads past the residual (the round-1 GenericShunt fix), the generic Stack, the crate's own iterator by value and by_ref, and chain/zip/extend of the crate's collections. The generated expected snapshots (committed in the diff) place the GenericShunt instantiations and the dropError argument where std would.
Quest docs: quest/m2/ts-model/README.md's census moves 69 → 82 refusals at the pin, and the from-iterator quest deletes.
What I verified on head 89f73a2, beyond reading:
GenericShunt parity above against rust-lang/rust master sources (fetched and quoted locally, not from memory).
lang::collects passes locally: cargo test --locked --test fixtures lang::collects (translates, typechecks, and differential-matches native Rust).
Both census claims, reproduced at pin ffcb7cd against the branch and against origin/main (a392514): model 93 → 82, and the normalized refusal-site sets differ by exactly the 11 collect/flat_map/flatten refusals (in model/origin.rs, model/subscription/runs.rs, auth.rs, moq-pattern's patterns.rs, and kio's SmallVeccollect) with no new ones; moq-net as a whole 434 → 409. Both counts are what the PR body and the quest README state.
A reviewfrog pass hunting for other std-generic paths that instantiate a crate's own FromIterator/IntoIterator found none left unsound after this PR: sum/product, partition, unzip, String/Cow collects, and box/ref/&mut receivers of the new collect branch either resolve to the crate's impl through the same gate or refuse loudly at body.rs's error path. Its one latent note (an own type that is both an own Iterator and has a custom IntoIterator) is a Rust coherence impossibility, so unreachable.
cargo clippy --locked --all-targets in ts/ is clean. CI (merge queue) had not started at review time.
The reason will be displayed to describe this comment to others. Learn more.
✅ No new issues found.
Reviewed changes — this run reviewed only the two commits since the last pullfrog review of 89f73a2d: 3a87d2fb merges base (main, fc56c13d) into the branch, and ae2e09fd regenerates the six lang/collects snapshots against it.
Merge of base: lang::collects's generated snapshots now reflect main's newer fmt-args lowering (formatters inlined straight into fmtFormat/panicFmt calls; plain objects instead of a labeled field for part-of-tuple uses). Pure generated output; the diff carries no source or runtime change.
test(ts): regenerate collects snapshots against base: exactly the mechanical UPDATE_EXPECT=1 regen the merge requires, six files (+46 -86).
Verified on head ae2e09fd: cargo test --locked --test fixtures lang::collects passes (translate + tsc + differential against native Rust), which is both the snapshot check and the semantic check for anything the merged base could have disturbed. The GenericShunt/collect/flat_map lowering reviewed last round is untouched (ts/src/lower/iter.rs and ts/runtime/rs/iter.ts are unchanged in the delta), and the census in the quest README was counted at the old head only for the changes this delta does not touch, so it still describes the branch's lowering.
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
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.
kixelated
changed the title
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Problem
std's iterator adaptors refused a crate's own collection:
collectinto a type with its ownFromIterator, andflat_map/flattenover a type with its ownIntoIterator. moq-net's model reaches these through moq-pattern'sPatterns(auth.rs,model/origin.rs,Patterns::rebase), rangemap'sRangeInclusiveSet(model/subscription/runs.rs), and smallvec'sSmallVec(kio'slog.rs).Approach
rs.Itermappings already exist for every adaptor; what was missing is std's generic code calling the crate's impl.collectinto the crate's own collection calls itsfrom_iter, instantiated for the receiver's type, with the receiver itself, as std'scollectdoes. Acollectinto aResultorOptionof one hands it theGenericShuntstd'stry_processdoes, which is anrs.Iter.into_iterof a crate's own collection calls itsinto_iterand iterates what that returns. That one place covers every adaptor taking anIntoIterator:flat_map,flatten,chain,zip,Vec::extend, andFromIteratorof std's collections.fn_value, so a binding or#[track_caller]impl is handled as anywhere else.Result/Optionshunts now read on past anErrorNone, as std'sGenericShunt::nextdoes, and a laterErrreplaces the earlier one, which then drops. Before this PR only std's collections read a shunt, and they stop at the firstNone. A crate'sfrom_itercan read again. That is whyiterCollectResultnow takes the error's dropper, and thetasksnapshot shows the extra argument.New fixture
lang/collectsruns against native Rust (505 cases, 0 differences). Its items log as they are made, read, and dropped, so the log shows laziness and drop order. It covers:collectthat stops early, leaving the rest of its source to drop;ResultandOptioncollects, including a collection that reads past the residual; a generic collection; a crate iterator by value and byby_ref;flat_mapread part way and thennext_back;flattenby value and by reference;flat_mapinto a type whose iterator is the crate's own and drops; andchain,zip, andextend.I mutation-tested it by editing the translated output and runtime by hand. Each mutation failed the run:
flat_mapResult's collection after anErrcollectsource&BagiteratorErrErrNoneCensus of
ts/moq/model.tomlat the current moq pin (ffcb7cd, bumped by #390): origin/main's driver counts 93 refusals and this branch counts 82, with exactly the 11 collect/flat_map/flatten refusals gone and no new ones. moq-net as a whole goes from 434 to 409. The epic now says 82; its old 69 was counted at the previous pin. Programs that don't use this translate as before: every other snapshot is unchanged buttask's extra argument, andjust ts moqpasses.Impact
FromIterator/IntoIteratortype, which translation used to refuse.rs.iterCollectResulttakes an optional error dropper.Alternatives
Iterator::collect/FlatMapMIR instead of thers.Itermappings: every other adaptor is mapped, and std's bodies here go through specialization andGenericShuntinternals.Follow-ups
util::idle::Idle'sOption<K>field asOption<()>(16), arrayvec'sArrayString(const generics,begin_panic,str::as_ptr),Weak::as_ptr,thread::yield_now, andBox::new_uninit_slice. The epic's breakdown of what remains predates the bump. Worth a/quest-planpass.Review
mechanical fix by codex / Codex:
Merged base and regenerated six stale collects snapshots; passed in ts/ on pinned nightly: UPDATE_EXPECT=1 cargo test --locked --test fixtures lang::collects -- --exact; cargo build --locked; cargo test --locked; tsc -p runtime.(head ae2e09f)Codex (
quest review), gpt-6.1-sol, medium effort:Triescollection inlang/collectsand verified by mutation.🤖 Generated with Claude Code