Conversation
| @@ -1,10 +1,10 @@ | |||
| ;(function(Array, TypeError, asyncIterator, defineProperty, iterator) { | |||
| ;(function(Array, TypeError, Symbol·asyncIterator, Object·defineProperty, Symbol·iterator) { | |||
| @@ -0,0 +1,210 @@ | |||
| ;(function(IteratorHelper, InternalError, TypeError, call, Symbol·iterator) { | |||
There was a problem hiding this comment.
A thing to keep in mind when reviewing this file: it's almost impossible to change or reorder anything here without breaking some test262 tests.
|
I'll do a full review shortly, bun in the meantime, some food for thought: do we want to have each of these as a separate file or maybe have a |
|
With a single .js file there's no lazy-loading of individual built-ins; it's all or nothing. |
Fair enough! |
bnoordhuis
force-pushed
the
iterator-zip-take2
branch
from
cd2a98a to
0ed8683
Compare
|
@saghul if you could find it in you to review this pr and the other one? |
| @@ -0,0 +1,210 @@ | |||
| ;(function(IteratorHelper, InternalError, TypeError, call, Symbol·iterator) { | |||
There was a problem hiding this comment.
| ;(function(IteratorHelper, InternalError, TypeError, call, Symbol·iterator) { | |
| ;(function(IteratorHelper, InternalError, TypeError, call, Symbol.iterator) { |
Was that a weird middle dot rather than a period?
There was a problem hiding this comment.
Ah I see now we are pasing the symbol straight. It mislead me, so may I suggest SymbolIterator ?
There was a problem hiding this comment.
I of course considered that when I wrote it but I don't like it because it obscures the fact that it's Symbol.iterator.
Or from a different angle: with SymbolIterator it becomes ambiguous if IteratorHelper actually refers to IteratorHelper or Iterator.Helper.
(Yes, academic now because there's no Iterator.Helper property but sooner or later something's going to show up that's actually ambiguous.)
There was a problem hiding this comment.
No strong opinion, land it then!
| if (item.done) break | ||
| let iter = item.value | ||
| check(iter, "bad iterator") | ||
| let method = iter[Symbol·iterator] |
There was a problem hiding this comment.
| let method = iter[Symbol·iterator] | |
| let method = iter[Symbol.iterator] |
| throw e | ||
| } | ||
| // note: uses plain numbers for |state|, using | ||
| // constants grows the bytecode by about 200 bytes |
There was a problem hiding this comment.
Ah, I assume this is why you wrote it so compact 😅
| let alive = count | ||
| return { | ||
| __proto__: IteratorHelper, | ||
| // TODO(bnoordhuis) shows up as "at next (<null>:0:1)" in stack |
There was a problem hiding this comment.
Did you mark it as TODO because it can be fixed?
There was a problem hiding this comment.
Yes, or at least, I think it's plausible that it can be fixed.
2 participants
I initially tried to do this in C but it was so onerous I decided to switch to JS. Marginally less onerous but still a massive PITA to implement.
Iterator.zipKeyedis up next.