Parse a dynamic function's parameter and body strings as wholes of their own - #3075
Merged
Merged
Conversation
lahma
force-pushed
the
staging/parser-early-errors
branch
3 times, most recently
from
August 20, 2026 09:02
09c5655 to
22bc989
Compare
…eir own
CreateDynamicFunction assembles function anonymous(<params>\n) {\n<body>\n} and
then parses the parameter string and the body string separately before parsing
that assembly, so that each half is valid alone. Jint only parsed the assembly,
so an argument string reaching across a boundary the assembly inserted produced
a function where the spec requires a SyntaxError.
Rather than pay two extra parses per new Function(...), the single parse is
checked against the two source positions the assembly fixed in advance: the
body's opening brace can only sit at its expected offset when the inserted
closing parenthesis is the one that ended the parameter list, and the parse can
only yield one statement when the body string did not close the function and
continue with statements of its own.
Frees staging/sm/Function/invalid-parameter-list.js.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lahma
force-pushed
the
staging/parser-early-errors
branch
from
August 20, 2026 09:16
22bc989 to
b66e7b6
Compare
lahma
added a commit
that referenced
this pull request
Aug 23, 2026
* Perform the internal methods the array built-ins name, not equivalents (#3066) The exclusion comment for these four staging files blamed Proxy trap invariants and said Jint's own defineProperty invariant check fires where it should not. That check is correct. Each file is a different defect, and one of them is not about Proxy at all. Array.prototype.concat step 6 is Perform ? Set(A, "length", n, true). Jint ended with a raw [[DefineOwnProperty]] of "length" carrying the array attributes (writable, non-enumerable, non-configurable). That bypasses a species result's own [[Set]], and on a Proxy whose target has an ordinary configurable "length" it is a genuine invariant violation - which is what the defineProperty check was correctly reporting. Array.prototype.slice step 12 is the same Set, and Jint performed no length write at all on its generic path, so a species result that is not an array came back with the wrong length. Concat step 5.b.iv.2 asks HasProperty per element and skips the CreateDataPropertyOrThrow for a hole. Jint discarded the answer and defined every index, so a hole in the source became an own undefined in the result. The question has to be asked afresh each iteration: an earlier element's CreateDataPropertyOrThrow can delete a later one. CreateArrayIterator steps 10.d.v-vi are a bare Get. Jint asked HasProperty first, an extra observable trap on a Proxy or an array-like host object, and read the element even for a key-kind step that has no use for it. A hole now also resolves through the prototype chain, which is what Get does. Array.prototype.filter and .map delegate their dense-receiver fast path to ArrayInstance, which had no callee realm to work with and built the result from the running one. Both now take the calling built-in's realm, so a cross-realm g.a.filter(...) produces a g-realm array. This is the same class of defect #3049 fixed elsewhere; those two paths were left behind because the receiver cannot supply the realm. Reflect/has.js is not a Proxy test. A String exotic object owns an index only for the canonical numeric string of that index - StringGetOwnProperty steps 3-6 - and Jint parsed the key with a plain ToNumber, so "01", "+1", "1.0", " 1", "1e0" and "-0" all resolved to a character the spec says is not there. ArrayInstance.IsArrayIndex is exactly the canonical test and is cheaper than the ToNumber it replaces. Frees staging/sm/Array/concat-proxy.js, staging/sm/Array/from_proxy.js, staging/sm/Array/species.js and staging/sm/Reflect/has.js. Refs #3021. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 139bc73) * Parse a dynamic function's parameter and body strings as wholes of their own (#3075) CreateDynamicFunction assembles function anonymous(<params>\n) {\n<body>\n} and then parses the parameter string and the body string separately before parsing that assembly, so that each half is valid alone. Jint only parsed the assembly, so an argument string reaching across a boundary the assembly inserted produced a function where the spec requires a SyntaxError. Rather than pay two extra parses per new Function(...), the single parse is checked against the two source positions the assembly fixed in advance: the body's opening brace can only sit at its expected offset when the inserted closing parenthesis is the one that ended the parameter list, and the parse can only yield one statement when the body string did not close the function and continue with statements of its own. Frees staging/sm/Function/invalid-parameter-list.js. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 9e95a9c) * Give a class its own source text and name the for-in head's anonymous function (#3078) Frees three staging/sm conformance files whose exclusion comment named only one of the three defects behind them. Function.prototype.toString must return "the source text matched by" the class production (https://tc39.es/ecma262/#sec-class-definitions-runtime-semantics-evaluation). A class with an explicit constructor already did; a class without one returned the native-code placeholder, because the engine substitutes a synthesized constructor AST that every such class shares and that node -- parsed apart from the script -- carries no source text. Class nodes are now stamped with the parse input when source-text retention is on, and the constructor's interpreter definition is keyed on the class BODY so each class gets its own (the class node itself is already a cache key: it is a field initializer's own value node, so `B = class {}` would collide). The exclusion comment blamed parenthesized class expressions and "some method forms"; parenthesization was never the problem and methods were already correct. GeneratorFunction and AsyncGeneratorFunction had a [[Prototype]] of their own .prototype object instead of %Function%, contrary to https://tc39.es/ecma262/#sec-generatorfunction-constructor and https://tc39.es/ecma262/#sec-asyncgeneratorfunction-constructor. AsyncFunction already had it right. That, not function source text, is what generators/runtime.js was failing on. The AnnexB for-in head is one of NamedEvaluation's positions (https://tc39.es/ecma262/#sec-runtime-semantics-forinofloopevaluation), so `for (var f = function () {} in {})` must name the function `f`; it produced "". Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 56a74c3) * Give Map and Set the [[SetData]] tombstone their traversals are specified over (#3073) Set.prototype.intersection and .isSubsetOf walk the receiver's [[SetData]] by index while the set-like they are handed is free to mutate that receiver from its has callback. The spec keeps the walk coherent by never removing an entry: a deleted one becomes ~empty~ in place, so every surviving entry keeps its index. Jint's ordered set compacted on delete instead, so a delete shifted later entries left underneath the walk and it skipped or repeated elements. The same List, and the same exposure, is behind Set.prototype.forEach, the Set iterator, difference, isDisjointFrom, and every one of Map's counterparts, so the representation is fixed once for both: KeyedCollectionData holds the entry list with tombstones and a key-to-slot dictionary, which also makes delete O(1) where it used to be a linear scan plus a linear shift. Deleted slots are reclaimed when the last entry is deleted and when an append finds at least half the slots dead, so add/delete churn cannot grow the list without bound; both reclaim paths move live entries, so a suspended cursor resumes by the entry's own sequence number rather than by a raw slot index. The five hand-written "adjust the position for mutations" heuristics that stood in for the tombstone are gone with it. So are two ordering defects they were no part of, both found while implementing. intersection, difference and symmetricDifference answered from an unordered hash set whose enumeration reuses the slot a delete freed, so after `s.delete(1); s.add(4)` they reported the re-added element first. And isSupersetOf read the receiver's size before GetSetRecord where step 4 reads it after, so a set-like whose size, has or keys getter grows the receiver was compared against a size taken before its own getters had run. Frees staging/sm/Set/intersection.js and staging/sm/Set/is-subset-of.js. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 7298417) * Fix four independent conformance gaps in the staging exclusion list (#3077) Iterator.from's wrapper no longer validates the wrapped next()'s result, GetIterator calls @@iterator with the primitive receiver, the global symbol registry is keyed by symbol identity rather than by description, and a string module export name is no longer confused with the namespace marker. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 048c0b0) * Fix argument validation and evaluation order in five built-ins (#3069) The TypedArray-from-ArrayBuffer constructor ran both of its observable coercions up front, so the misaligned-offset RangeError of step 3 was reported after ToIndex(length) of step 5 rather than before it, and it narrowed byteOffset and length to Int32 before the bounds checks, so a value past 2^31 wrapped into one that passed them. The array iterator kept stepping its closure after it had completed, so a buffer detached once the iterator was exhausted turned the next `next` into a TypeError. `super[e]` resolved its super base before evaluating `e`. Number.prototype.toFixed coerced its receiver instead of requiring a Number. Intl.PluralRules left its localeMatcher option unvalidated. Frees six staging/ exclusions. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 26c2137) * Settle Atomics.waitAsync timeouts from the event-loop pump (#3095) A finite waitAsync timeout was a Task.Run whose Task.Delay resumed on a second thread-pool dispatch, so settling a one-millisecond wait needed two threads from a pool that injects roughly one every 500ms once it is saturated. test262's built-ins/Atomics/waitAsync/* family gives itself a fixed wall-clock lifespan (~1000ms) and polls for the outcome on the engine thread — which is free the whole time — so on a loaded two-core runner the pool, not the engine, missed the budget and the test reported its own $DONE('Test timed out'). The engine's logic was never wrong. The timeout is now a deadline on an engine-owned registry that the pump reads, and the thread pool is out of the settlement path entirely. - AtomicsWaiterDeadlines (Jint/Native/Atomics/AtomicsWaiterDeadlines.cs): a min-heap keyed on (Stopwatch timestamp, sequence), engine-thread only and lock-free, hung off Engine._atomicsWaiterDeadlines — null until the first finite-timeout wait and again as soon as the last one leaves, which is what keeps the pump's check to a single null test. A wait asking for no timeout registers nothing. A deadline on a monotonic timestamp cannot fire early, which is what the deleted re-delay loop existed to reconstruct: test262 asserts the observed lapse is at least the timeout asked for. - The check runs once per pump iteration, not only at queue exhaustion, and that is the one place this deliberately differs from the timers contract. test262's $262.agent.setTimeout is a promise chain rather than a timer, so a test polling for its own waitAsync keeps the job queue permanently non-empty and a deadline consulted only at exhaustion would never be consulted at all — it would turn a flake into a hard failure. Ordering is untouched: settling only enqueues the resolution, behind everything already queued. - Hot-path footprint, per target framework. All five: one predictable null test per event-loop job (Engine.SettleTimedOutAtomicsWaiters, aggressively inlined), zero on the engines that never register such a wait, and one Stopwatch.GetTimestamp per pump pass only while one is pending. net8.0+: the exhaustion-time timer check is unchanged. net462/netstandard2.0/netstandard2.1: TryPromoteDueTimerJob is now declared there too and folds to a constant false, so the pump's generated code is what it was — the #if left EventLoop, DrainEventLoopUntil and AwaitPromiseSettlementAsync entirely and moved into Jint/Engine.Pump.cs. - Both idle waits now bound themselves by TimeUntilNextPumpScheduledWork, the earlier of the next waiter deadline and (net8.0+) the next due timer, so the sub-10ms clamp and the IsRunningJob guards the timers PR put in place now serve atomics on every target framework. - The notify/timeout race is guarded exactly as before: the compare-and-swap in AsyncWaiter.Resolve decides, and the timeout route leaves the waiter list before it claims, so a notify racing it counts the wait as gone rather than waking one about to report timed-out. The heap reads the settled flag without synchronization on purpose — a stale false costs one extra pass, nothing more. - ResetTransientEvaluationState drops the deadlines beside _webApi?.ResetTransientState(). The waits stay in their blocks' waiter lists, exactly as a wait asking for no timeout always has, so what Atomics.notify counts no longer depends on whether the old pool task had happened to fire yet. Honest limitation: the flaking tests check elapsed time before they check outcomes, so a full engine-thread stall longer than the test's lifespan still fails them and no engine change can prevent that. What this removes is the pool-starvation mechanism, which is the one actually observed. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit ad32403) Backport note (4.x): 4.x has no opt-in web APIs, so the timer half of the original is absent here and Engine.Pump.cs carries the Atomics half alone — TryPromoteDueTimerJob is not declared, TimeUntilNextPumpScheduledWork is just the next waiter deadline, and Jint/Engine.WebApi.cs does not exist. The conditional compilation the original moved into Engine.Pump.cs therefore has nothing to guard and is gone with it. EventLoop.RunAvailableContinuations is restructured to `while (true)` with the settle call ahead of the dequeue, which is exactly the shape the original produced minus the timer promotion, and the bounded EventLoop.WaitForEventAsync(TimeSpan, CancellationToken) overload — a timers-PR addition that 4.x never received — is brought across with it, because Engine.AwaitPromiseSettlementAsync needs it to bound an idle wait by the next deadline. Two pre-existing `<see cref="WaitForEventAsync"/>` references became CS0419-ambiguous once that overload existed and are qualified, as they are on main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S * Never read a pending lazy name descriptor when rendering an error message (#3114) GetOwnFunctionNameForMessage read _nameDescriptor?.Value under a doc comment claiming the read touches the raw field and never invokes an accessor. That is false for a descriptor carrying PropertyFlag.CustomJsValue: Value routes through the CustomValue accessor, and a script function's own name starts as the shared pending-lazy sentinel whose accessors throw by design. Rendering an error message for a function whose name was never materialized could therefore itself throw InvalidOperationException - a latent throw inside error-message construction, the one place that must never produce one. The method now skips any CustomJsValue descriptor and falls back to the CLR type name, which is also the right answer for the message's purpose: only a plain materialized string was ever quoted. The doc comment is corrected to say why. Closes #3112 Claude-Session: https://claude.ai/code/session_011G7Ud48VAs1JicxRZxLDxg Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 9f1d9fa) * Propagate a rejected iterator return() out of an abandoned for-await loop (#3113) AsyncIteratorClose (https://tc39.es/ecma262/#sec-asynciteratorclose) calls the iterator's return(), Awaits what it answered (step 4.d), and only then ranks that against the loop's own completion: a throw completion already in flight wins (step 5), otherwise a rejected return() becomes the loop's completion (step 6), and a settled value that is not an Object is a TypeError (step 7). for-await-of performed the synchronous IteratorClose instead. That calls return(), sees the promise it answered with, finds that it IS an Object and stops — so steps 4.d through 7 never ran at all. A return() that rejected was dropped on the floor and `for await (...) { break; }` completed normally where V8 throws; a return() answering Promise.resolve(42) passed an Object check that should have been made against the settled value; and a plain sync iterable under for-await lost its close failures wholesale, because %AsyncFromSyncIteratorPrototype%.return reports a throwing sync return(), a non-object result and a rejected `value` alike as a rejection of the promise it hands back. The close now suspends the surrounding async function or async generator on that Await like any other one: IteratorInstance.TryStartAsyncClose performs steps 3 and 4.a-4.c, the settlement re-enters the statement, and steps 5-8 decide the loop's completion there. A break, a return and a jump naming an enclosing label all take it, both from the ordinary exit and from the one a per-iteration `await using` dispose resumes into. A throw completion deliberately keeps the synchronous close. Step 5 hands that completion back whatever the close does, so the outcome is already the spec's, and the throw reaches the loop's finally as a CLR unwind that could only be suspended by catching it — the very thing the loop's exception filter exists to avoid. What is given up is confined to the microtask the discarded Await would have taken. Which completions close at all is unchanged: a step that fails and an iterator that runs out still close nothing (#3047). Fixes #3098 Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 2cd8a7f) * Run FinalizationRegistry cleanup on the event loop, and stop two algorithms severing themselves across jobs (#3135) Two of the three defects #3125 records, both about work reaching the engine from somewhere the single-thread contract does not cover. `FinalizationRegistry` called its cleanup callback directly from `~Observer()`, i.e. on the CLR finalizer thread, and `CleanupFinalizationRegistry` was an empty stub. The finalizer now does two thread-safe things and nothing else — push the cell onto a concurrent queue and enqueue a generation-stamped event-loop job — and the callback runs from the pump, on the engine's thread, with the cell's `[[HeldValue]]` as its argument, which it never used to receive at all. That path could not actually fire, because the token index was a strong `Dictionary` keyed by the *possibly undefined* unregister token, so every cell's sentinel was retained forever and no target's collection was ever observed. Both tables are now `ConditionalWeakTable`s, and which one holds which object is load-bearing: the sentinel lives only in the target-keyed table, so a live token cannot keep it alive, and an empty token indexes nothing. The sentinel's handle on the registry is weak — otherwise a live target keeps the whole engine alive, and dropping the registry reports collections that never happened — and is the registry's own field rather than one per sentinel, because `WeakReference<T>` is itself finalizable and a private one is already-finalized as often as not by the time the sentinel's finalizer reads it. The generation stamp is the cell's registration generation, per cell rather than per registry, and the drain re-checks it per cell because one job may reach cells from more than one cycle. Separately, five continuations re-enqueued through a raw `AddToEventLoop(Action)` from inside a reaction job — one in `for await` inside an async generator, four in `Array.fromAsync` — severing an in-flight algorithm across jobs the specification does not have. Both now continue inside the reaction job their `Await` already owns, which is what `Await` is specified to do (https://tc39.es/ecma262/#await) and what the async-function branch next to the generator one already did. `Array.fromAsync`'s handler pairs become engine-internal `IPromiseContinuation`s through a new `PromiseOperations.UponPromise`, shared with the streams code that had the same reaction class privately. The re-enqueue was commented "to prevent stack overflow"; it was not what prevented one, and `ArrayFromAsyncDoesNotRecurPerElement` pins that with 20,000 elements. Observable microtask counts shift by one job per loop turn, pinned by `AlgorithmJobBoundaryTests`; the full test262 suite is unchanged. (cherry picked from commit 60f41b4) Backport note (4.x): the one-line hunk in Jint/WebApi/Streams/StreamPromises.cs is dropped — 4.x has no web APIs, so there is no streams code to move onto the shared PromiseOperations.UponPromise. Everything else applies unchanged; the new VoidPromiseContinuation is core and Array.fromAsync is its only consumer here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S * Park an already-resolved computed key across an await or a yield (#3133) (#3144) `o[f()] = await g()` called `f` twice, and so did `({ [f()]: await g() })` — once before the suspension and once on the replay — so a key function with side effects ran twice and, when it produced a different name each time, the value landed under the second call's key while the first stayed absent. Both shapes behaved identically through a generator's `yield`. Two sites, one mechanism (the suspendable's SuspendDataDictionary, which the compound forms `+=`/`-=`/… have used correctly all along): - `SimpleAssignmentExpression.SetValue` resolved the left-hand Reference, then on suspension returned it to the reference pool and discarded it, leaving the replay to re-evaluate the whole left-hand side. It now parks it in `AssignmentSuspendData.Lref` and consumes it on resume, exactly as `JintAssignmentExpression.EvaluateMaterialized` does — including the pooling discipline: the suspendable owns the Reference until the resume completes, which is what returns it and clears the entry. That covers the base as well as the key, so `b()[k()] = await x` and `a[i++] = await x` each run their side effects once too. - `JintObjectExpression.BuildObjectNormal` parked Target/NextIndex but not the key of the property whose *value* suspended, so the resume re-ran the key expression and its ToPropertyKey conversion (itself user-visible). `ObjectExpressionSuspendData.PendingKey` now carries the converted key, written on every save so a suspension at another index cannot inherit it, and only ever for a computed key — a static one costs nothing to re-derive. Nothing is parked unless a suspension actually happens, so the non-suspended path allocates exactly as before. `SetValue` in fact loses an execution-context read: the suspended/aborted checks now sit behind the single `Suspendable` null test they both imply. Claude-Session: https://claude.ai/code/session_011G7Ud48VAs1JicxRZxLDxg Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 15dcc5d) * Keep a computed property key's handler identity across a suspension (#3150) A suspension inside a computed key — `({ [f() + await g()]: 1 })` — lost every scrap of replay state and re-ran the whole key subtree, so `f` was called twice and the property landed on whatever the second call produced. `AstExtensions.TryGetComputedPropertyKey` called `JintExpression.Build` on every evaluation, handing the key expression a brand-new handler instance each time. Everything a suspension parks for its replay — `LeftOperandSuspendData`, `AdditionChainSuspendData`, the argument buffers — is keyed on the handler that parked it, so a key expression that is a different object on the way back can never find its own parked state. Issue #3133 fixed the sibling case (the key had finished and the *value* suspended, parked as `ObjectExpressionSuspendData. PendingKey`); this is the case nothing above the key can park, because the key never produced a value. Every key position now takes its handler from a per-engine, node-keyed cache (`Engine.GetOrBuildPropertyKeyExpression`) instead of building a throwaway one. Engine-owned rather than published to the AST's `UserData` for the same reason `_functionDefinitions` is: a handler tree accumulates engine-affine inline caches and must not be shared across engines. That covers every route into a key at once — object-literal properties, methods and accessors, class methods, fields, static members and auto-accessors, and destructuring patterns, whose static `HandleObjectPattern` has no handler of its own to hang one on. Steady state gets cheaper, not dearer: a computed key now costs one dictionary lookup where it used to cost a full recursive `JintExpression.Build` and the allocations under it, and the handler stays warm across evaluations. One shape needed a second fix. A method is defined from inside `MethodDefinitionEvaluation`, before the object literal's own suspension check runs, so the placeholder undefined a suspended key produced was converted and defined as a property literally named "undefined" — on the very object the resume carries forward, which therefore could never take it back. It now bails on `ExecutionContext.IsSuspended`; the pre-existing sibling check next to it uses `Suspended`, which only sees a generator parked at a yield and would have missed the await shape entirely. Claude-Session: https://claude.ai/code/session_011G7Ud48VAs1JicxRZxLDxg Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit ee74aa5) * Call stack: a nested evaluation's unhandled throw trims to its entry depth instead of clearing (#3214) `ScriptEvaluation` called `ResetCallStack()` on both of its unhandled-throw paths, which clears the whole stack. When a host callback re-enters the engine mid-run — the browser-host "dynamically inserted script" shape — the nested evaluation sits above frames that belong to an outer run which is still live, and clearing took those with it: the outer run's balanced pops desynced (tolerated only by the `TryPop` guards), and `Error`s it constructed afterwards captured truncated traces. The evaluation now captures `CallStack.Count` at entry and pops back to it through a new `JintCallStack.TrimTo`. At a top-level entry the entry depth is 0, so the stack ends where `Clear()` left it; for the bookkeeping the two differ, and the difference is the point. `Clear()` wiped `_statistics` and issued `RecordAbandon()`; popping decrements per frame and issues one `RecordExit` each, so the outer run's frames stay open in a profile — which is honest, because they are. Tail-call retention is unaffected: the `catch` sits outside every trampoline, so `ReleaseTailRetention` has already run before the trim. `SetJavaScriptCallstack` still runs before the trim, so the thrown exception's own trace is unchanged. `Advanced.ResetCallStack()` keeps clearing outright. One consequence beyond stack traces: the outer run's `MaxRecursionDepth` budget is no longer silently reset by a nested evaluation's unhandled throw. That was a hole in the limit of the same family as the tail-call one, and closing it is the correct direction. The test fails on the unfixed code on both TFMs — the outer stack came back missing `at outerFunction` — and passes with the fix. (cherry picked from commit cd5bb38) * Array.prototype.values/keys/entries: no array-like gate on the receiver (#3236) https://tc39.es/ecma262/#sec-array.prototype.values is two steps -- ToObject(this) then CreateArrayIterator(O, value) -- and neither reads `length`. The read belongs to the iterator: step 1.b of its abstract closure is LengthOfArrayLike, a Get plus a ToLength, performed afresh on every next(). All three of `values`, `keys` and `entries` instead gated on ObjectInstance.IsArrayLike, which demands a `length` that is present, is *already* a JsNumber, and is non-negative, and threw `TypeError: cannot construct iterator` otherwise. That is wrong in six ways at once: absent means 0, a string or a boolean coerces, a negative clamps to 0, and a throwing `length` getter belongs to the first next() rather than to values(). So `[...Array.prototype.values.call({})]` was a TypeError where the specification says `[]`, and `[...{[Symbol.iterator]: Array.prototype.values}]` was one too. The fix is to drop the gate. Nothing else had to move: ArrayIteratorPrototype's array-like iterator already performs the per-next() length read the specification asks for, through ArrayOperations' ObjectOperations lane, whose GetLength() is exactly `ToInteger(Get(O, "length"))` clamped at zero. The dense-array lane is untouched by construction -- Construct still routes a JsArray to ArrayIterator on a type test, and ToObject hands an ObjectInstance straight back, so the for-of hot path is one virtual IsArrayLike call *lighter* than the gate that used to precede it; a test pins the lane so that stays true. Typed arrays keep the ValidateTypedArray their own %TypedArray%.prototype.values performs, and so does the array-like iterator, because CreateArrayIterator step 1.b.i asks for it whenever the receiver has [[TypedArrayName]]. ArrayIteratorReceiverTests covers the seven shapes the issue names plus the ordering the gate hid: a Proxy proves values() performs no property access at all (neither a `get` nor the `getOwnPropertyDescriptor` the old TryGetValue probe fired) and that the first next() performs exactly a Get of `length` and then one of the index; a counting getter reads 0/1/2/3/3 across values(), three next()s and one past exhaustion; the length is re-read per step, so a receiver that grows between two steps yields what it grew by and one that shrinks below the current index completes there; ToLength coerces rather than rejects (NaN, 'abc', {}, [], undefined, -Infinity, -0.5, false all being zero, 2.9 and ' 2 ' and '0x2' and ['2'] all being two); and a throwing getter, a throwing valueOf and a Symbol `length` all erupt from next() rather than from values(). Holes resolve through the prototype chain in both lanes, `keys` reads no element, and `entries` yields a pair for every index including the absent ones. Found by the FileAPI web-platform-tests corpus (#3208), where `new Blob({[Symbol.iterator]: Array.prototype[Symbol.iterator]})` is precisely this shape; it reproduces with no web API enabled at all, which is what made it an engine finding rather than a Blob one. The three NeedsTriage rows of Blob-constructor.any.js go with it, and the driver's rule -- an entry must match at least one failing test -- is what now enforces the fix. Blob-constructor.any.js is 73-for-73. Fixes #3209 Claude-Session: https://claude.ai/code/session_01JmK1CzJSgMd1LyTEMX3yiE Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 5e4c61a) Backport note (4.x): the three Jint.Tests/Wpt hunks are dropped — 4.x has no web-platform-tests area, so there is no exclusion table to shrink and no corpus-inventory row to rewrite. The engine change and its own tests apply unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S * Destructuring: the rest of an exhausted array is empty, not 2^32 elements (#3263) `var [x, ...rest] = []` handed back an array whose length was 4294967295. Not merely reported, either: `JSON.stringify(rest)` raised `RangeError: Invalid string length`, `[...rest]` raised a CLR `OutOfMemoryException` and `rest.concat(...)` an `IndexOutOfRangeException`, none of which a script can catch — so an ordinary `const [first, ...rest] = row; return JSON.stringify(rest);` threw a hard error on an empty row. BindingRestElement and AssignmentRestElement both build the rest array from the remainder of the iterator — "Repeat, while iteratorRecord.[[Done]] is false" — so once the leading elements have exhausted the source there is nothing left to add and the result is `[]`: https://tc39.es/ecma262/#sec-runtime-semantics-iteratorbindinginitialization https://tc39.es/ecma262/#sec-runtime-semantics-iteratordestructuringassignmentevaluation Jint short-circuits the iterator when the source is array-like with the original array iterator, and copies the tail by index instead. That lane sized the tail as an unsigned `length - i`. A pattern with more leading elements than the source has leaves `i` past `length`, so the subtraction wrapped and `ArrayCreate` was handed a length near 2^32. Every shape whose head over-consumes was affected — `[a, b, ...r] = [1]`, `[x, ...r] = []`, `[, , ...r] = [1]`, the assignment form, nested patterns, for-of and catch parameters — and the deficit scaled, so five targets over an empty source produced 4294967291. The fix is the clamp: `i < length ? length - i : 0`. That comparison is the one the copy loop below already makes on its first iteration, so the lane pays nothing it was not paying; the `j - i` inside the loop needs no clamp because `j` starts at `i`. Rest *parameters* were never affected: `FunctionEnvironment. HandleRestElementArray` does the same subtraction in `int` and clamps it before the cast, and a parameter's array pattern routes through there too. The general iterator lane counts up from zero and was correct as well, which made it the oracle: a 176-shape differential sweep (each pattern run against both a real array and a generator yielding the same values, all compared to node v24) showed all 74 divergences to be this one wrap, and none left after the fix. test262 does not cover the shape. Its `ary-ptrn-rest-id-exhausted.js` is `var [, , ...x] = [1, 2]` — exactly exhausted, `i == length`, which does not wrap. Counts are unmoved: 102495 passed / 0 failed / 189 skipped, identical to a baseline run of the same suite on this machine. Fixes #3249 Claude-Session: https://claude.ai/code/session_01JmK1CzJSgMd1LyTEMX3yiE Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 74570cb) Backport note (4.x): the hunk conflicted because main's adjacent line already read `(uint) Math.Min(arrayOperations.GetLongLength(), ArrayOperations.MaxArrayLength)` where 4.x still had `arrayOperations.GetLength()`. Taken as written on main. That is a small behaviour change on this branch, not a no-op. ObjectOperations .GetLength() here is an unchecked double->uint narrowing with no upper bound: var integerLength = GetIntegerLength(); // double return (uint) (integerLength >= 0 ? integerLength : 0); while GetLongLength() immediately beneath it clamps with Math.Min(integerLength, MaxArrayLikeLength). Out of range that conversion is unspecified, so the target frameworks disagree: measured on a length of 2^53, the old expression gives 4294967295 on net10.0 and 0 on net472 (x64 - being unspecified it is not stable across architectures either), where the new one gives 4294967295 on both. On net472 that silently produced an *empty* rest array; it now behaves as .NET 10 already did. What reaches this is narrower than it looks, and narrower than an object literal. The lane is gated on `obj.IsArrayLike && obj.HasOriginalIterator`, and ObjectInstance.HasOriginalIterator is hard-coded false, so `[...rest] = {length: 2**53}` is not iterable at all and never arrives. The receiver has to be an ObjectWrapper over a CLR type TypeDescriptor calls array-like but that ArrayOperations.For does not route to IndexWrappedOperations - one implementing ICollection<T>/IReadOnlyCollection<T> without the non-generic ICollection - whose "length" script has then shadowed with Object.defineProperty(wrapper, 'length', {value: 2**53}). A plain `wrapper.length = 2**53` is swallowed by the CLR property and does not. This is a deliberate path-local fix, not a backport of #3248, which deletes the uint overload of LengthOfArrayLike across the public surface and is excluded from this stack: nothing is deleted here, no other call site moves, and one call site stops depending on a conversion whose result is undefined. It opens no new denial-of-service door either - a length of exactly 2^32-1 already drives the loop below four billion times on every target framework. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: David Jeske <davidj@gmail.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
1 participant
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.
Part of #3021 (wave 2). This workstream owned six
staging/exclusions filed under one commentsaying "Parser early errors that Acornima does not raise, or raises as the wrong error type".
That diagnosis was wrong in both directions, so the triage came first. One file is Jint's and is
fixed here; the other five are defects in Acornima, the external parser, and stay excluded with
prose that names the actual defect and what would remove it.
The one that was Jint's —
staging/sm/Function/invalid-parameter-list.jsNot a parser problem at all. CreateDynamicFunction
assembles
function anonymous(<params>\n) {\n<body>\n}(steps 14-15) and then parsesparamStringand
bodyParseStringon their own (steps 17-20) before parsing that assembly as a singlefunction expression (steps 23-24). The spec's own note names the case:
Jint parsed the assembly only, and took
Body[0]from the resulting script. So an argument stringthat reached across a boundary the assembly inserted produced a working function. Before this change:
Four of the test's five cases plus the trailing-statement case; the fifth (
"//",") {") alreadyfailed because the assembly itself does not parse. V8 rejects all of them.
What changed
Jint/Native/Function/FunctionInstance.Dynamic.cs— reparsing both halves would cost two extraparses on every
new Function(...)that misses the compilation cache, so the existing single parseis checked against the two source positions the assembly fixed in advance
(
ValidateDynamicFunctionShape, with the reasoning in its doc comment):{can only sit atprefixLength + parameterLength + 3when the)the spec insertedafter the parameters is the one that closed the parameter list, so the text the parser accepted as
parameters is exactly the parameter string;
continue with statements of its own.
The two per-kind switches collapsed into one that yields just the source prefix — the zero-argument
branch built a string character-identical to what the general branch builds, and one construction
site means one place the expected offset is derived from. The stale spec step numbers in the two
comments there are updated to the current edition's.
Evidence
Pre-fix, with the file un-excluded:
Post-fix:
Passed! - Failed: 0, Passed: 2for that file, andFailed: 0, Passed: 1115, Skipped: 5acrossbuilt-ins/Function,built-ins/GeneratorFunction,built-ins/AsyncFunction,built-ins/AsyncGeneratorFunctionandstaging/sm/Function;Failed: 0, Passed: 2255acrosslanguage/eval-code,language/expressions/function,language/statements/functionandlanguage/directive-prologue.Jint.TestsandJint.Tests.PublicInterfaceare green on both target frameworks.Jint.Tests/Runtime/DynamicFunctionSourceTests.csadds 20 cases (10 of which fail against theunfixed engine with "Expected Jint.Runtime.JavaScriptException, but no exception was thrown"),
covering the five test262 pairs, body strings that continue past the function, the rejection
surviving the compilation cache's second-sighting promotion, each function kind's own source prefix,
and parameter strings that legitimately contain comments, parentheses, nested function bodies,
templates, patterns, trailing commas, rest elements and line feeds.
The five that are Acornima's
None of these can be fixed in this repository; the exclusion comments are rewritten to say so
precisely. Each was reproduced against Acornima 1.7.0 (the pinned version) and cross-checked against
V8, which accepts or rejects each one as the spec requires.
class/derivedConstructorArrowEvalSuperCall.js,class/derivedConstructorArrowEvalNestedSuperCall.js,fields/bug1587574.js— aSuperCallinside a direct eval. Jint already parses eval code withAllowSuperOutsideMethod, but that coversSuperPropertyonly; Acornima 1.7.0 exposes no separateswitch for a direct
super(), soeval("super()")in a derived constructor fails to parse with'super' keyword unexpected here. PerformEval forbids itonly when
inDerivedConstructoris false, which Jint already computes inEvalFunction. Thenon-eval halves of those files (
(()=>super())(),(()=>(()=>super())())()) already pass.adams85/acornima#44 adds
ParserOptions.AllowDirectSuperOutsideMethodand is still a draft — onceit lands and ships, this is a package bump plus one line in
EvalFunction.fields/bug1587574.jsis the same defect, not "a field initializer's TDZ" as the old comment said:its
class Q { f = 1; }exists only to defeat SpiderMonkey's lazy parsing, and theReferenceErrorit expects is the derived constructor's
thisTDZ, raised because the eval'dsuper()sits insidearrows that are never called.
destructuring/bug1396261.js— not "a destructuring pattern with a duplicate binding". Acornimadoes not re-raise the
CoverInitializedNameearly error when an object literal carrying ashorthand-with-initializer ends up as the object of a member expression inside a destructuring
target.
[{a = 0}] = []is valid because{a = 0}is refined into anObjectAssignmentPattern, butin
[{a = 0}.x] = []the element is the member expression{a = 0}.x, so{a = 0}stays anObjectLiteraland 13.2.5.1applies: "
PropertyDefinition : CoverInitializedName— It is a Syntax Error if any source text ismatched by this production." Jint accepts all four such forms; the file's valid half already passes.
strict/deprecated-octal-noctal-tokens.js— Jint does not raise "the wrong error type" here; thefirst half of the file already passes with the right one. What fails is the second half: Acornima
scans the token immediately after an ASI-terminated
"use strict"directive before it applies thedirective, so a legacy octal literal or a legacy-octal/
\8/\9escape appearing in exactly that onetoken escapes the strict-mode early errors of
12.9.3.1 and
12.9.4.1. The strictness itself is
applied correctly —
with, duplicate parameters andarguments =are all caught — and moving theliteral one statement further on, or terminating the directive with an explicit semicolon, restores
the error. Only the sloppy run fails; in the strict run the harness's own prologue makes the
tokenizer strict from the first character. Not yet reported upstream.
🤖 Generated with Claude Code