1197095 - ForOfIterator passes undefined to next() calls

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[André Bargull [:anba]]

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"

Comment 1

[Morgan Phillips [:mrrrgn]]

Giving this a go.

Comment 2

[Morgan Phillips [:mrrrgn]]

Attachment: forof.diff

So, this change was extremely tiny but seems to do the trick without breaking tests.

Comment 3

[Morgan Phillips [:mrrrgn]]

Attachment: forof.diff

Had left a random commented out line of source in the old patch.

Comment 4

[Tom S. (please needinfo tschuster)]

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.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/786516f51bc3

Comment 6

[Morgan Phillips [:mrrrgn]]

Attachment: forof.diff

With amended test, as per evilpie's suggestion. Thx for the review. :)

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/786516f51bc3