Repository navigation
update Iterator.prototype.{take,drop} for new RangeError - #5065
Merged
Merged
Conversation
michaelficarra
force-pushed
the
iterator-counter-numbers
branch
from
May 28, 2026 03:08
469477b to
4038d43
Compare
linusg
approved these changes
Jul 9, 2026
ljharb
approved these changes
Jul 9, 2026
ljharb
added a commit
to es-shims/iterator-helpers
that referenced
this pull request
Jul 10, 2026
…angeError when `limit` is finite and > 2 ** 53 - 1 Per tc39/ecma262#3776 and tc39/test262#5065, `take`/`drop` now throw a RangeError (closing the underlying iterator) when `limit` is a finite Number greater than `2 ** 53 - 1`; `Number.MAX_SAFE_INTEGER` and `Infinity` remain valid. The redundant `remaining !== Infinity` closure guard is dropped (the amended spec decrements unconditionally, since `Infinity - 1` is `Infinity`), and the polyfill feature-detect now requires this behavior.
bakkot
approved these changes
Jul 16, 2026
Member
|
Several approvals already, merging. |
ptomato
force-pushed
the
iterator-counter-numbers
branch
from
July 29, 2026 16:16
07be9dd to
fad1e93
Compare
bakkot
enabled auto-merge (squash)
July 29, 2026 17:30
Member
|
Actually, I'm not sure whether to merge this first or wait for tc39/ecma262#3776 |
Member
|
Whoops, sorry, didn't see your comment in time. That said: normative ecma262 PRs usually (but not always) wait for implementations to land before merging, and implementations don't usually start until after tests land, so I don't think it makes to wait for the ecma262 PR. We should land tests as soon as we have consensus on the behavior asserted in the tests (as we do in this case). |
3 of 5 tasks
lahma
added a commit
to sebastienros/jint
that referenced
this pull request
Aug 1, 2026
Moves the pinned suite from defaaf15 to b363f29d. Nine upstream commits land, and one of them is normative: tc39/test262#5065 restructured Iterator.prototype.take/drop so that a *finite* limit above 2**53 - 1 is now a RangeError, checked before ToIntegerOrInfinity. Six tests that passed at the old pin fail at the new one without an engine change, so the bump is not just a pin change. The restructured algorithm also forms its Iterator Record before validating the argument, which means a validation failure closes the receiver through "return" without ever reading "next". Jint already did exactly that via the pre-record IteratorClose overload, so only the missing step needed adding; the comments are renumbered to the new step numbering to keep them checkable against the spec. Infinity stays a valid limit - the step only rejects a finite one. AsyncIterator.prototype.take/drop share a limit validator that had the same gap. test262 has no AsyncIterator directory, so nothing upstream pins it and the two validators agreeing is what keeps them from drifting. double.IsFinite arrived in .NET Core 3.0 / netstandard2.1, so it is backfilled in the existing extension(double) block for net462 and netstandard2.0. That lets the guard read the way the spec step is phrased on every target framework. The two brand-new features in this range - iterator-chunking and iterator-includes, 122 test files between them - are parked in ExcludedFeatures until their implementations land, so this bump stays green on its own. Suite regenerates to 99,654 cases, of which 99,494 pass and 0 fail. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
faik
added a commit
to robomotionio/goant
that referenced
this pull request
Aug 1, 2026
…e to green (#3) * Add the iterator helpers that graduated into the specification test262 moved `iterator-chunking` and `iterator-includes` below its `## Standard language features` header, so these stopped being proposals we could skip and became work: 109 tests appeared as failures with no change on our side. - Iterator.prototype.includes(searchElement [, skippedElements]). skippedElements is deliberately not coerced — a non-Number throws without consulting valueOf — and running the source dry is not a close, so `return` is never read. - Iterator.prototype.chunks(chunkSize) and .windows(windowSize [, undersized]). chunks and windows are the only helpers that accumulate their input, so they need a buffer the collector can see. A Go slice will not do: append past capacity reallocates and holdCaptures only ever recorded the original header, so everything after the first growth would be invisible to a trace. The buffer is a JS array reached through a one-element slot mutated in place, the same trick flatMap uses for its inner iterator. Verified under GOANT_GC_POISON=1 with GOANT_GC_FLOOR=64. Also implements the normative change in tc39/test262#5065: a take/drop limit that is finite and above 2**53-1 is now a RangeError rather than a silently unbounded one. +∞ still means "no limit". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Dispose a module's top-level using resources when evaluation ends compileVarDecl only registers a disposer when c.usingStack >= 0, which means an enclosing block or function body has already set up a disposal stack. A module body is neither, so `using x = resource` at the top level of a module bound the name and then never disposed it — the six initializer-*disposed-at-end-of-module tests hung rather than failed. Wrap a program body in the same scaffolding a function body uses. It is emitted below hoistEnd deliberately: for a Module everything above that point is InitializeEnvironment and runs at LINK time, and the disposal stack has to be created when the module is evaluated instead. Scripts get it too. A Script has no top-level await, so the parser has already rejected `await using` there and only the synchronous disposal ops can be reached. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Score the whole suite too, so the core profile stays auditable -core is our own name for our own exclusion list; test262 publishes no profiles. It was excluding 10,835 of 53,575 files while the README described it in one sentence, which is not enough for anyone to check. The largest single exclusion is Temporal, and calling it a staged proposal was simply wrong — test262 files it under standard features. It is out on scope, not on staging, and it is worth about ten points. -all disables both skip lists so the number has no judgement calls in it. The two reconcile: 44,691/53,411 unfiltered, whose 8,720 failures are 8,718 in the excluded buckets, one load-induced timeout, and one genuine gap (tco-fn-realm), which is also the single -core failure. Iterator.prototype.join joins the skip list. It sits above test262's `## Standard language features` header alongside import-defer and source-phase-imports, which are already there. Not deriving the list from that header automatically, tempting as it is: Temporal sits below it, so deriving would quietly pull in 4,611 tests. The header is still the thing to watch — a proposal crossing it turns skippable tests into work, which is exactly how a 2-failure score became 138 without goant changing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add a Go-idiomatic API for embedding goant The only way into the engine was v8go/, a V8-shaped interface we inherited from the runtime we replaced and kept for compatibility. It stays. This adds the API we would have written if there had been nothing to be compatible with. Runtime/Program/Value/Object/Function/Promise, ToValue and Export for reflection-based conversion in both directions (json tags, embedded structs flattened the way encoding/json does it), Func for exposing Go functions, typed errors, WithContext for deadlines. Two deliberate departures from goja, whose choices here are known footguns: a number always exports as float64 rather than int64-or-float64 depending on the value, and a property read returns (Value, error) because a getter or a Proxy trap can throw and swallowing that is how you get a silent wrong answer. Value is a struct, not an interface, so it costs no allocation. Scope and Pool expose the invocation model, which nothing outside v8go could reach: a fresh global whose prototype is the shared one, 111 ns against 366 µs for a whole realm, and Release rewinds the allocator so a run's memory goes back without a trace. Pool retires a runtime that a job polluted, hit the memory limit, was interrupted, or panicked through. embed.go grows the primitives the above needs (arrays, dates, bigints, zero-copy Uint8Array, construct, symbol/typed-array predicates) and is still the one file promising a stable surface to a host program. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Write the README, and make goja a column in the benchmarks Where goant came from (ant's C engine on Hacker News, Bun's Zig-to-Rust rewrite suggesting C was close enough to Go to try), why it exists at all rather than robomotionio/v8go, why not goja, what it is, and how to use it. The numbers are measured, not quoted. A dedicated Azure D8s_v5 with no other workload, non-burstable so credits cannot skew a run, torn down afterwards. That was worth doing: the laptop Octane figures we had been carrying were wrong — Splay had the sign flipped. bench/gojarun is a separate module, so goja stays out of goant's dependency graph, presenting the same CLI contract as ./goant. That is what lets both harnesses drive it, including goant-t262 -runner, whose flag had been there all along and had never been pointed at another engine. It turns "goja is behind on modern JS" from an assertion into 33,303/42,740 on the same profile and checkout. The Octane harness now writes its scratch file as .cjs. Octane is 2013-era sloppy-mode code and crypto.js assigns an implicit global, a ReferenceError under the module goal. deno and bun treat .js as a module and node does not, so the old extension scored the choice of goal rather than the engine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Put the goant mascot at the top of the README Resized to 640px wide from the 1024x1536 original: the header renders at 300px, and 2.2 MB is a lot to carry in history forever for an image nobody sees at full size. 341 KB instead. Lossless recompression alone only bought 4%, so it had to be the pixels. Left untrimmed on purpose — the transparent margin is not dead space, and cropping to it clips the antennae. The background is transparent rather than dark, so it sits correctly in both GitHub themes; the white "go" keeps its dark outline on light backgrounds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Clear the faint wash that drew a box around the logo The mascot's background was not transparent. It carried a light grey wash — around 1-3% alpha at the edges, peaking near 14% behind the character where it was meant to read as a glow. At that opacity it is invisible as a tint but its rectangular boundary is not, so the README showed two faint vertical seams down either side of the image. Easy to miss when checking: a threshold test at 5% steps straight over the band that causes it, and reports the background as already clean. Probe the corner pixels instead. Alpha is levelled 20%-100% on the 1024x1536 original before the resize, so the cut lands on the full-resolution glow rather than on interpolated edges. A ramp rather than a hard threshold, so the glow fades out instead of leaving a ring where it crosses the cut. Corners are now exactly (0,0,0,0) and the artwork is untouched: 26.91% of pixels opaque against 26.94% before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add black and white placements of the logo The README uses the transparent original, which is right there because it follows the reader's GitHub theme. These two are for the places that cannot do that — a slide, a conference page, a badge on someone else's site — where handing over a PNG with a known background beats hoping the transparency lands on something that suits it. Same asset centred on a solid canvas with even padding. Both were checked rather than assumed: the wordmark's "go" is white, and it stays legible on white because the dark outline carries it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
tc39/ecma262#3776 updates these two methods to throw a RangeError during parameter evaluation for very large finite Number values.
While I was in there, I fixed up the metadata for each of the files I touched. If that's unwanted, I can revert it.
Disclaimer: I used AI in the process of creating this PR.