Closed
Bug 1021835
Opened 8 years ago
Closed 6 years ago
for-of should throw TypeError if the @@iterator method returns a non-object value
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: jorendorff, Assigned: arai)
References
(Blocks 1 open bug)
Details
Attachments
(4 files)
7.84 KB,
patch
|
evilpie
:
review+
|
Details | Diff | Splinter Review |
4.15 KB,
patch
|
till
:
review+
|
Details | Diff | Splinter Review |
2.93 KB,
patch
|
evilpie
:
review+
|
Details | Diff | Splinter Review |
2.00 KB,
patch
|
evilpie
:
review+
|
Details | Diff | Splinter Review |
Spotted by André Bargull. See bug 904723 comment 28. // Primitive values can't be iterators. var a = {}; a[std_iterator] = () => 0; Number.prototype.next = () => ({value: undefined, done: true}); assertThrowsInstanceOf(() => Array.from(a), TypeError);
Reporter | ||
Updated•8 years ago
|
Summary: Array.from should throw TypeError if the @@iterator method returns a non-object value → for-of, Array.from, etc. should throw TypeError if the @@iterator method returns a non-object value
Comment 1•6 years ago
|
||
It seems like most code now uses either GetIterator (self-hosted) or JS::ForOfIterator (C++), both check for primitive return values. So we really just need to emit some extra byte-code to disallow primitives in for-of. Number.prototype.next = () => ({value: undefined, done: true}); for (let x of {[Symbol.iterator]() { return 0 }}) {}
Updated•6 years ago
|
Summary: for-of, Array.from, etc. should throw TypeError if the @@iterator method returns a non-object value → for-of should throw TypeError if the @@iterator method returns a non-object value
Assignee | ||
Comment 2•6 years ago
|
||
will post patch after test
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•6 years ago
|
||
There are several places that does GetIterator, and separated patch for each with Bytecode (Part 1) * array destructuring * spread * for-of * yield* ForOfIterator::init (Part 2) * Promise.all * Promise.race GetIterator self-hosted function (Part 3, Part 4) * Array.from * %TypedArray%.from * %TypedArray% ctor Inlined for better error message with decompiling (Part 4) * Map ctor * Set ctor * WeakMap ctor * WeakSet ctor in Part 1, added JSOP_CHECKISOBJ after emitIterator.
Attachment #8810010 -
Flags: review?(evilpies)
Assignee | ||
Comment 4•6 years ago
|
||
in ForOfIterator::init, replaced ToObject with isObject+toObject, to avoid converting primitive to object. then, to test Promise.all/race, moved helper functions from js/src/tests/ecma_7/AsyncFunctions/shell.js to js/src/tests/shell.js.
Attachment #8810011 -
Flags: review?(till)
Assignee | ||
Comment 5•6 years ago
|
||
in GetIterator, JSMSG_NOT_ITERABLE is used to report invalid iterator, that is wrong. replaced it with JSMSG_NOT_ITERATOR, that says "iterator" instead of "iterable".
Attachment #8810012 -
Flags: review?(evilpies)
Assignee | ||
Comment 6•6 years ago
|
||
just added testcases.
Attachment #8810013 -
Flags: review?(evilpies)
Comment 7•6 years ago
|
||
Comment on attachment 8810011 [details] [diff] [review] Part 2: Do not convert iterable[@@iterator]() value to object in ForOfIterator::init. Review of attachment 8810011 [details] [diff] [review]: ----------------------------------------------------------------- Nice, r=me
Attachment #8810011 -
Flags: review?(till) → review+
Updated•6 years ago
|
Attachment #8810010 -
Flags: review?(evilpies) → review+
Updated•6 years ago
|
Attachment #8810012 -
Flags: review?(evilpies) → review+
Updated•6 years ago
|
Attachment #8810013 -
Flags: review?(evilpies) → review+
Assignee | ||
Comment 8•6 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d27d2fec192e62ec05267147c60769b6379bf2e4 Bug 1021835 - Part 1: Emit JSOP_CHECKISOBJ after GetIterator in byte code. r=evilpie https://hg.mozilla.org/integration/mozilla-inbound/rev/9b186ff95a5ef9ff00237fa72b19a0aa2d620a6d Bug 1021835 - Part 2: Do not convert iterable[@@iterator]() value to object in ForOfIterator::init. r=till https://hg.mozilla.org/integration/mozilla-inbound/rev/953d2b8f17036a81990d82eac213622191019b8e Bug 1021835 - Part 3: Use "iterator" instead of "iterable" in the error message for iterator. r=evilpie https://hg.mozilla.org/integration/mozilla-inbound/rev/7b4af0b1626b70d331c0769a91faa3f72a876dc2 Bug 1021835 - Part 4: Add testcases for primitive iterator. r=evilpie
Assignee | ||
Comment 9•6 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e366a3e90d2f87c8466dd9edf19581029023659 Bug 1021835 - followup: Skip test that needs drainJobQueue in jsreftest. r=bustage
Comment 10•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d27d2fec192e https://hg.mozilla.org/mozilla-central/rev/9b186ff95a5e https://hg.mozilla.org/mozilla-central/rev/953d2b8f1703 https://hg.mozilla.org/mozilla-central/rev/7b4af0b1626b https://hg.mozilla.org/mozilla-central/rev/0e366a3e90d2
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•