1315641 - Change error messages in self-hosted GetIterator function

Status: RESOLVED FIXED

(Core :: JavaScript: Standard Library, enhancement)

Description

[André Bargull [:anba]]

The self-hosted GetIterator function should use the same error messages for invalid iterators as used by other parts of the engine:

---
js> new Map({[Symbol.iterator]: 1})
typein:1:1 TypeError: ({[Symbol.iterator]:1}) is not iterable
Stack:
  @typein:1:1
js> ([] = {[Symbol.iterator]: 1})
typein:2:8 TypeError: ({[Symbol.iterator]:1}) is not iterable
Stack:
  @typein:2:8
js> Int8Array.from({[Symbol.iterator]: 1})
typein:3:1 TypeError: number is not a function
Stack:
  @typein:3:1
---

Expected: Int8Array.from() reports "... is not iterable".
Actual: Int8Array.from() reports "... is not a function"


And:
---
js> new Map({[Symbol.iterator]: () => 1})
typein:1:1 TypeError: number is not a non-null object
Stack:
  @typein:1:1
js> Int8Array.from({[Symbol.iterator]: () => 1})
typein:2:1 TypeError: 1 is not iterable
Stack:
  @typein:2:1
---

Expected: Int8Array.from() reports "... is not a non-null object".
Actual: Int8Array.from() reports "... is not iterable"

Comment 1

[André Bargull [:anba]]

This issue needs to be re-evaluated after the changes from bug 1147371.

Comment 2

[Julien Cristau [:jcristau]]

Mass wontfix for bugs affecting firefox 52.

Comment 3

[André Bargull [:anba]]

Attachment: bug1315641.patch

- Inlines GetMethod in ArrayFrom and TypedArrayStaticFrom, so we can throw a custom error message (JSMSG_NOT_ITERABLE) instead of the default JSMSG_NOT_FUNCTION.
- Changes the error message in GetIterator to JSMSG_GET_ITER_RETURNED_PRIMITIVE and removes the now unused JSMSG_NOT_ITERATOR.
- And since ArrayFrom uses for-of iteration, we don't actually need the extra GetIterator call, which improves the following µ-benchmark from 165ms to 145ms for me:
    var items = [];
    var t = dateNow();
    for (var i = 0; i < 1000000; ++i) {
        Array.from(items);
    }
    print(dateNow() - t);

Comment 4

[André Bargull [:anba]]

Attachment: bug1315641.patch

I forgot to update this assertion (http://searchfox.org/mozilla-central/rev/5696c3e525fc8222674eed6a562f5fcbe804c4c7/js/src/builtin/Array.js#846) in ArrayFrom to handle |null|. But in the end I decided to simply replace the assertion with a comment similar to the one in TypedArrayFrom (http://searchfox.org/mozilla-central/rev/5696c3e525fc8222674eed6a562f5fcbe804c4c7/js/src/builtin/TypedArray.js#1487-1488).

Comment 5

[André Bargull [:anba]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9bdad241dea901c0008faec4df1065f2f85edde9

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/711cba275460
Align error messages for (Typed)Array.from with standard iteration error messages. r=evilpie

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/711cba275460