ReviewOS

zig-utils/zig-js

Iterator protocol: make GetIterator require and honor @@iterator

#752
Closed chris opened this 9 days ago ·
9 days ago

Parents: #475, #493 Related: #750, #751

Problem

The shared iteratorOf entry point is more permissive than ECMA-262 GetIterator:

  • an ordinary object with a .next property but no Symbol.iterator is accepted as iterable;
  • generator objects are returned before their observable Symbol.iterator property is read, so an override, getter, deletion, or non-callable value is ignored.

A current-main probe returns false,1,1,0 for [manualObjectThrew, manualNextCalls, overriddenGeneratorSum, iteratorGetterCalls]; the specified result is true,0,7,1. This affects consumers such as Math.sumPrecise even though their subsequent IteratorStep behavior is correct.

Scope

  • Make the language GetIterator path perform ordinary GetMethod on @@iterator, require it to be callable, call it once with the input as receiver, and require an Object result.
  • Honor observable overrides/deletions on generator and other built-in iterator objects.
  • Keep deliberate GetIteratorDirect/GetIteratorFlattenable consumers distinct.
  • Audit every iteratorOf caller for the correct abstract operation and retain one captured [[NextMethod]] per iterator record.
  • Preserve intrinsic array/string/Map/Set iteration behavior without changing their iterator storage or next implementations.

No-workaround rules

  • No .next-property duck typing for GetIterator, generator/builtin bypass, array-only shortcut that skips observable @@iterator, repeated method lookup, source rewriting, or silent fallback after a missing/non-callable method.
  • No stack, queue, job, Promise, Worker, or Map/Set iterator implementation changes.

Acceptance

  • Plain .next objects are rejected without calling next; an explicit @@iterator makes them iterable.
  • Generator/builtin @@iterator getters, overrides, deletion, non-callable values, receiver, and result validation are observed in spec order.
  • Every iteratorOf caller is classified as GetIterator, GetIteratorDirect, or GetIteratorFlattenable and captures [[NextMethod]] once where required.
  • Focused units, affected Test262 iterator/builtin subtrees, and full integration gates pass with exact nonzero accounting and zero unrelated flips.
9 days ago

First staged slice is complete locally: strict GetIterator record construction plus Math.sumPrecise migration. The adversarial probe changes from false,1,1,0 to specified true,0,7,1; focused Math units are 4/4, 0 leaked; exact-parent Test262 Math.sumPrecise is 10 pass -> 10 pass, 0 flips. Commit is intentionally held from main until the current descendant matrix finishes its unaffected gates; that matrix’s only observed failure so far is the separately tracked #722 case.

9 days ago

Second staged slice is complete locally: Object.fromEntries now uses strict GetIterator plus its captured [[NextMethod]], rejects object-tagged Symbol/BigInt primitives, and preserves the primary entry TypeError across IteratorClose. Focused regression: 1/1, 0 leaked. Exact-parent Test262 test/built-ins/Object/fromEntries: 25 pass -> 25 pass, 0 flips. Both #752 commits remain held while the existing main matrix finishes its unaffected long-running gates.

9 days ago

Third staged slice is complete locally: WebAssembly.Tag parameter and WebAssembly.Exception payload sequences now require @@iterator and reuse captured [[NextMethod]]. Focused Wasm API regression: 1/1, 0 leaked. The checked-out Test262 corpus contains 0 Tag/Exception files; later CI Wasm smoke jobs are the integration evidence. All three commits remain held while the existing main matrix completes.

9 days ago

Three strict-GetIterator slices are now on main:

Exact evidence remains: Math.sumPrecise 10 -> 10 pass, Object.fromEntries 25 -> 25 pass, 0 flips in both; focused regressions 4/4, 1/1, and 1/1 respectively, all with 0 leaked. Main integration: CI 32808460795. The broader caller-classification acceptance item remains open; this intentionally does not touch Promise, Worker, stack/queue/job, or Map/Set iterator implementation paths.

9 days ago

Spread syntax is now strict on main in 32ca9e3f fix(spread): require strict iterator records (#752). Array, call, and constructor spread require @@iterator, reuse one captured [[NextMethod]], and no longer direct-copy packed arrays while skipping observable %ArrayIteratorPrototype%.next lookup/calls.

Evidence: exact regression 1/1, complete spread unit filter 13/13, 0 leaked. Exact-parent Test262: array 52 -> 52 pass, call 91 -> 91 pass with the same pre-existing tco-non-eval-with.js failure, and new 59 -> 59 pass; 0 flips across 202 passing cases. Descendant integration: CI 32809472046.

9 days ago

Compiled sync iteration is strict on main in e6bfc95e fix(vm): require iterables for iterator bytecode (#752). VM iter_of now performs observable @@iterator acquisition and Object validation, while the compiler remains the single owner of [[NextMethod]] capture for compiled for-of, array destructuring, and yield*—avoiding a double next read.

Evidence: focused required-bytecode regression 1/1; related unit filters for-of 6/6, destructuring 20/20, yield-star 3/3, all 0 leaked. Exact-parent Test262: for-of 751 -> 751, yield 63 -> 63, expression generator destructuring 186 -> 186, statement generator destructuring 186 -> 186; 1,186 pass -> 1,186 pass, 0 flips. Descendant integration: CI 32810113953.

9 days ago

Tree-walker sync iteration is strict on main in 84b2b84b fix(interpreter): require iterator records in loops (#752). Synchronous for-of and array destructuring now use one strict Iterator Record; packed-array destructuring no longer skips observable iterator methods. The broader filter exposed and this commit also fixes missing anonymous default-function name inference on the general destructuring path.

Evidence: focused tree-walker regression 1/1; for-of units 6/6; complete destructuring filter 20/20, all 0 leaked. Exact-parent Test262: for-of 751 -> 751 plus five destructuring families 670 -> 670; 1,421 pass -> 1,421 pass, 0 flips. Descendant integration: CI 32811738762.

9 days ago

AggregateError is strict on main in 4776abb7 fix(error): require AggregateError iterables (#752). Its IteratorToList path now requires @@iterator and invokes the captured [[NextMethod]] directly, so .next-only inputs are rejected without execution and a captured non-callable next cannot fall back to a later lookup.

Evidence: complete AggregateError unit filter 2/2, 0 leaked. Exact-parent Test262 test/built-ins/AggregateError: 25 pass -> 25 pass, 0 flips. Descendant integration: CI 32812098999.

9 days ago

TypedArray object construction is fixed on main in 503b7ee4 fix(typedarray): reuse selected iterator method (#752). The constructor now reuses its original GetMethod(@@iterator) result and one captured [[NextMethod]]. If @@iterator is absent, the specified array-like path remains intact; a .next-only array-like is indexed and next is never called.

Evidence: focused constructor filter 4/4, 0 leaked. Exact-parent Test262 object-argument constructors: Number family 28 -> 28, BigInt family 31 -> 31; 59 pass -> 59 pass, 0 flips. Descendant integration: CI 32812474909.

9 days ago

Internal cursor factories are classified and fixed on main in 64817d65 fix(iterator): build internal cursors directly (#752). Array.prototype.values plus snapshot-backed URLSearchParams, Headers, and FormData iterator methods now construct their cursors directly instead of incorrectly dispatching through mutable Array.prototype[Symbol.iterator].

Evidence: affected local units 4/4, 0 leaked. Exact-parent Test262 test/built-ins/Array/prototype/values: 12 pass -> 12 pass, 0 flips. The three Web API factories have 0 Test262 files. Descendant integration: CI 32812744376.

9 days ago

Intl.ListFormat is strict on main in 9c682655 fix(intl): capture ListFormat iterator records (#752). StringListFromIterable now requires @@iterator, reuses one [[NextMethod]], validates iterator result Objects, and preserves its primary non-string TypeError when IteratorClose return() throws.

Evidence: affected ListFormat units 2/2, 0 leaked. Exact-parent Test262 test/intl402/ListFormat: 81 pass -> 81 pass, 0 flips. Descendant integration: CI 32812997510.

9 days ago

Caller audit after 9c682655: every non-reserved iteratorOf consumer is now migrated or classified as an internal cursor. The only remaining calls are deliberately inside the user-reserved boundary:

  • addEntriesFromIterable for Map/Set constructors — language GetIterator;
  • async-from-sync fallback — language GetIterator plus async iteration lifecycle;
  • Object/Map groupBy collection — language GetIterator in the Map/Set area;
  • Promise combinators and Promise.try iterable handling — language GetIterator in the job/Promise area.

No remaining iteratorOf call is a GetIteratorDirect/GetIteratorFlattenable consumer; those already have separate entry points. I am leaving the caller-classification acceptance box unchecked because the reserved callers still use the permissive legacy helper and still need their own captured [[NextMethod]] migrations. I will not cross that ownership boundary.

9 days ago

Follow-up audit found and closed the separate GetIteratorDirect/GetIteratorFlattenable record-retention defect as #753 in 9d67ec5. That change does not touch this issue's reserved Map/Set or Promise/jobs callers.

CI accounting for descendant 9c682655: run 32812997510 completed with 44 successful jobs and one functional failure, the existing async frame-unwind assertion tracked in #727. Eight remaining jobs were cancelled when #753 landed. The strict iterator-record unit tests passed before that unrelated failure.

The final #752 acceptance item remains open because its remaining callers are still inside the explicitly reserved Map/Set and Promise/jobs boundary.

9 days ago

Follow-up #754 is complete in fix(iterator): preserve VM close ordering (#754). The fixed-seed forced-tier matrix now guards strict iterator records across tree-walker and required VM, and it corrected two adjacent VM IteratorClose ordering defects without changing this issue’s remaining reserved Promise/Map/Set callers.

9 days ago

Implemented and pushed in daac3cc4 (fix(iterator): retire permissive GetIterator fallback (#752)).

The remaining Map/Set constructors, groupBy operations, Promise combinators/race, and Async-from-Sync fallback now use strict Iterator Records: a single observable GetMethod, a captured [[NextMethod]], direct next calls, precise GC roots, and IteratorClose on abrupt completion. This also removes the generator/packed-array/next-property shortcuts and the proxy-visible async has preflight.

Validation:

  • focused adversarial iterator-record units: 10/10 Debug, 10/10 ReleaseSafe, 10/10 unsuppressed TSan; 0 leaks
  • full test-parallel -Dunit-jobs=4: 2236 passed, 1 skipped, 0 failed, 0 leaked (four shards avoided the host storage pressure seen with ten concurrent temp trees)
  • affected test262 profiles: 2833 pass -> 2833 pass, 0 failures, 0 flips (Map 204; Set 383; WeakMap 141; WeakSet 85; Object.groupBy 14; Promise 677; Array.fromAsync 95; for-await-of 1234)

Fixes #752 is in the commit body.

Sign in to comment on this issue.