Closed Bug 1197095 Opened 4 years ago Closed 4 years ago

ForOfIterator passes undefined to next() calls

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox43 --- affected
firefox45 --- fixed

People

(Reporter: anba, Assigned: mrrrgn)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

Test case:
---
new Map({
  [Symbol.iterator]() {
    return this;
  },
  next() {
    print("args.length", arguments.length);
    return {done: true};
  }
});
---


Expected: Prints "args.length 0"
Actual: Prints "args.length 1"
Blocks: es6
Assignee: nobody → evilpies
Giving this a go.
Assignee: evilpies → winter2718
Attached file forof.diff (obsolete) —
So, this change was extremely tiny but seems to do the trick without breaking tests.
Attachment #8695084 - Flags: review?(evilpies)
Attached patch forof.diff (obsolete) — Splinter Review
Had left a random commented out line of source in the old patch.
Attachment #8695084 - Attachment is obsolete: true
Attachment #8695084 - Flags: review?(evilpies)
Attachment #8695127 - Flags: review?(evilpies)
Comment on attachment 8695127 [details] [diff] [review]
forof.diff

Review of attachment 8695127 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/tests/ecma_6/Map/iterable.js
@@ +12,5 @@
> +    };
> +    return iterator;
> +  }
> +};
> +

I would do more similar to Andre's testcase
let length;
let iterator = {
   [Symbol.iterator]() { return this; }
   next() { length = arguments.length; return {done: true}; }
}

let m = new Map(iterable);
assertEq(length, 0);

The only kind of important thing here is to test length after we invoked Map, otherwise the test would still succeed even when for whatever reason we stop calling next.
Attachment #8695127 - Flags: review?(evilpies) → review+
Attached patch forof.diffSplinter Review
With amended test, as per evilpie's suggestion. Thx for the review. :)
Attachment #8695127 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/786516f51bc3
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.