ReviewOS

zig-utils/zig-js

Map/Set/WeakMap/WeakSet constructors close an iterator that threw from its own next/done/value

#883
Closed glennmichael123 opened this 10 days ago ·
10 days ago

The one remaining test262 failure: staging/sm/Map/constructor-iterator-close.js. Pre-existing — it reproduces on ea3ff5b4 and earlier, and is unrelated to the tier work around it.

zig-out/bin/test262 --diag test/staging
  fail_runtime  sm/Map/constructor-iterator-close.js : Expected SameValue(«true», «false») to be true

The direction matters: iterable.closed is true where the test expects false. We call the iterator's return() in cases where the specification does not.

Three scenarios fail, each across all four constructors (12 assertions total):

casewhat throwsfile
next() itself throwsES2017 23.1.1.1 step 8.a:265
the done getter throwsstep 8.c:270
the value getter throwsstep 8.c:280

All three are the same rule: when the abrupt completion comes from the iterator's own next/done/value, the iterator record is marked done and is not closed. IteratorClose is for abrupt completions the consumer produces — a non-object entry, a throwing entry getter, a throwing set/add — and those cases all pass here already.

In current spec terms, AddEntriesFromIterable calls IteratorStepValue(iteratorRecord), which on an abrupt completion sets iteratorRecord.[[Done]] to true and propagates the throw without closing.

Reproducer

var it = { closed: false };
it[Symbol.iterator] = function () {
  return {
    next: function () { throw "next throws"; },
    return: function () { it.closed = true; return {}; }
  };
};
try { new Map(it); } catch (e) {}
print(it.closed);   // zig-js: true    node v24: false

Swap next for { value: {}, get done() { throw "x"; } } or { get value() { throw "x"; }, done: false } for the other two.

node v24 passes the complete unmodified file (all cases, no output from an assert shim). The consumer-caused close paths in the same file are correct here, so the fix is narrow: stop routing the iterator's own next/done/value throw through IteratorClose in the Map/Set/WeakMap/WeakSet constructor path.

10 days ago

Fixed in 5f88724.

Cause was a single blanket guard: addEntriesFromIterable wrapped its whole loop in errdefer if (!done) iteratorCloseKeepingThrow(...), so every abrupt completion closed — including the ones the iterator itself produced. IteratorStepValue marks the record done and propagates those without closing; only consumer-caused completions close, starting at "next is not an Object".

Object.fromEntries already had this right and documents the rule — only the Map/Set path used the blanket errdefer. The consumer-caused closes are unchanged and still keep the original throw; they are now attached explicitly to addOneEntry, covering a non-object entry, a throwing 0/1 getter, a throwing key coercion and a throwing adder.

test/staging/sm/Map is now 8/8 (was 7 pass / 1 fail), and full test262 is 48,506 / 48,506 valid, 4,669 / 4,669 negative — zero parse, runtime or host failures. The corpus has no remaining failures of any kind.

Sign in to comment on this issue.