Repository navigation
feat(ts): read zips by index as std does, and flatten's held inner iterators - #446
Conversation
…e they end Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ect in place translate Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou'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 28 minutes. View limit details
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. CodeRabbit Verify
|
|
quest land: this pull request conflicts with a pull request queued ahead of it, in these files:
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. |
|
codex / Codex land fixer note:
(written by codex / Codex) |
|
codex / Codex land fixer: Conflicts with the base: quest/m3/ts-drop-order.md, ts/fixtures/reject/census.md, ts/fixtures/reject/expected.txt, ts/fixtures/task/expected/drops.ts, ts/fixtures/task/src/drops.rs, ts/fixtures/task/src/harness.rs, ts/fixtures/task/ts.toml, ts/src/lower/iter.rs, ts/tests/task.ts (written by codex / Codex) |
Take's and Skip's folds keep main's back trim read once per fold, with Take's by-index front; Take no longer caches its fold's view, whose back now holds per-fold state. collect_take returns the in-place source's element size. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
ts-drop-orderlisted three refusals where std's drop order was one the runtime couldn't follow:skipof a zip of twoTrustedRandomAccessiterators of items that drop (reject fixtureskipped_zip).collectin place of such a zip whose closures own values that drop (zip_in_place).flattenorflat_mapwhose inner iterators hold items once they end.Approach
Flatten. The refusal is deleted.
FlattenFoldalready modeled std'stry_fold, which keeps an ended inner iterator until the next one replaces it. The one bug wasTakeIter.folded's back-trim shortcut, whichtake(1).rfindreached directly. std'sTake::try_rfoldreadsnth_back(len - n - 1)and drops the result, wherenext_backreadsnth_back(len - n).Zips by index.
rt.Itergainsindexed(), a view of the iterator as std reads it by__iterator_get_unchecked. The quest called this "a reader by index besidefolded". A view'sadvanceBackBypasses items without reading them, as index reads never touch them. Each reader by index goes through it:next,nth, andnext_back, and so its folds.TrustedRandomAccessNoCoercezip'sfold, nowrt.Trusted"index", distinct from aTrustedLenzip's"len".take's TRAspec_fold:iterTakegains anindexesflag. The flag is set only whereMAY_HAVE_SIDE_EFFECTholds, since otherwise the read is the same.collectin place of a NoCoerce zip:rt.Take"index"reads the zip by index, then drops it whole, inner closures first.Which adaptors override
indexed():Map: never callsffor an item it passes.EnumerateandFuse: pass the view through.Skip: makes and drops each skipped item whole as it reads index 0, from either end.The rule "
indexed() !== thisiff the iterator may have side effects" mirrorsMAY_HAVE_SIDE_EFFECT.It also fixes two pre-existing miscompiles that sat outside the refusals:
skipof a non-zip, read by index from the back, never made its prefix. std makes and drops it at index 0.SkipIter.foldeddrops its shortcut while it still skips, because std'stry_foldisnth(n - 1)plus a whole read, which differs fromnth(n)for a TRA zip.Impact
Translation: both zip refusals and the flatten refusal are gone, and the reject fixture loses
skipped_zipandzip_in_place. Only the B-treecollectis left in the quest. feat(ts): drop the closures std's consumers and entries own where std's do #438 removes the closures item.Public API: additive runtime surface only, no
MIGRATE.mdentry. That surface isIter.indexed(),"index"inrt.Trustedandrt.Take, anditerTake's optionalindexes. Generated output changes where a NoCoerce zip now passes"index"and a side-effecting TRAtakepassestrue.Tests: two new task steps, both differential against native wasm32:
flattened, 10 scenarios.indexed, 9 scenarios: by index from the front, the back,nth,takefolds, NoCoerce folds, adaptors, uneven sides, and in-place collects.Mutation checks: I reverted each runtime piece in turn, 17 in all; every one fails the task fixture. The lang fixture caught a bug the task fixture couldn't: a short-circuited
dropA?.(a.next())that skipped the read where nothing drops.Speed: on Bun, a micro-measure of the old runtime against the new over 4096 bytes, the new is no slower:
The codec bench gains
iter/zip-map-filter-count, a forward by-index zip of mapped sides. The browser bench (just ts bench) was not run.Alternatives
at(i)random-access read on every TRA class: about 20 std types, and more surface. The view keeps sequential iterators and overrides only the six adaptors whose by-index read differs.Follow-ups
None new. The B-tree
collectstays with the sort port.Review
mechanical fix by codex / Codex:
Merged base and regenerated conflicting rejection snapshots without behavior changes; passed in ts/ with pinned nightly: UPDATE_EXPECT=1 cargo test --locked --test fixtures reject -- --exact; UPDATE_EXPECT=1 cargo test --locked --test fixtures census -- --exact; cargo test --locked; cargo fmt --all --check; tsc -p runtime; tsc --noEmit --strict --target esnext --module esnext --moduleResolution bundler --allowImportingTsExtensions --verbatimModuleSyntax --skipLibCheck bench/iter.ts bench/workloa(head 5ec48e0)Local
quest review: Codex, gpt-6.1-sol, medium effort. Round 1 on head3ae00f48ddf548d0d5e869cdb7c4fe493594e6f4: no findings.Checks:
just cipasses locally on3b4b9313.3ae00f48adds only the quest's PR link (quest checkok).Merged origin/main through #462 and #459 (
iter.rs's zip and flatten helpers now live inlibrary.rs;SkipIter.foldedtakes main's version). Localquest reviewround 1 ona50060d9: an indexedTakefold read past its iterator's size (skip(4).take(1).foldmade and dropped four pairs; native made none). Reproduced in the task fixture'sindexedstep 3, fixed by clamping the limit to the iterator's length as std'sspec_folddoes. Round 2 ond68f0adc: no findings.just cipasses locally ond68f0adc.(written by Claude Opus 5.5)
🤖 Generated with Claude Code