ReviewOS

zig-utils/zig-js

Iterator helpers: retain GetIteratorDirect next-method records

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

Problem

The synchronous iterator-helper implementation captured next values, but two boundaries discarded or reinterpreted the capture:

  1. iterStepM(iterator, next_method) only called the captured value when it was already callable. A captured non-callable value fell back to callMethod(iterator, "next", ...), which performed a second observable lookup and could call a replacement method instead of throwing for the original [[NextMethod]].
  2. getIteratorFlattenableObj eagerly read iterator.next but returned only the iterator object. flatMap, zip, and zipKeyed therefore lost the GetIteratorDirect record; flatMap immediately read next again, while joint-iteration helpers stepped through dynamic next lookups.

This violated the iterator-helpers proposal's GetIteratorDirect / GetIteratorFlattenable contracts and created an attacker-controlled re-entrancy surface through accessors and proxies.

Required implementation

  • Represent direct/flattenable acquisition as an iterator record containing both [[Iterator]] and the exact captured [[NextMethod]].
  • Make stepping with a captured method call that exact value unconditionally; a non-callable captured value throws TypeError without consulting iterator.next again.
  • Preserve the record through lazy flatMap, Iterator.zip, Iterator.zipKeyed, and longest-mode padding.
  • Keep genuinely dynamic internal cursors separate and explicit rather than encoding "no captured method" as JavaScript undefined.
  • Add adversarial tests for getter/proxy lookup counts, replacement-after-first-read, non-callable captures, abrupt completion, and close ordering.
  • Run focused unit coverage and the relevant Test262 Iterator helper subtrees with before/after counts.

Constraints

  • No duck-typing fallback and no repeated property lookup after capture.
  • No workaround, proposal-test exclusion, or special case for built-in iterators.
  • Do not change Map/Set iterator implementation or Promise/microtask/Worker lifecycle as part of this issue.

Relationships

  • Follow-up to #752's strict language-level GetIterator work.
  • Advances the iterator/builtin correctness and adversarial-complexity tracks in #475 and #493.
9 days ago

Implemented and pushed to main in 9d67ec5.

Evidence:

  • focused Iterator unit filter: 8/8 passed, 0 skipped, 0 failed, 0 leaked;
  • exact-parent Test262 test/built-ins/Iterator: 514 passes before -> 514 passes after, 0 failures and 0 regressions;
  • adversarial coverage proves non-callable captures cannot trigger a second getter, and flatMap, concat, zip, zipKeyed, and padding retain the first captured next value.

No Map/Set iterator implementation or Promise/microtask/Worker lifecycle code changed.

Sign in to comment on this issue.