Skip to content

Conversation

@bnoordhuis
Copy link
Contributor

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.zipKeyed is up next.

@@ -1,10 +1,10 @@
;(function(Array, TypeError, asyncIterator, defineProperty, iterator) {
;(function(Array, TypeError, Symbol·asyncIterator, Object·defineProperty, Symbol·iterator) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cute, innit?

@@ -0,0 +1,210 @@
;(function(IteratorHelper, InternalError, TypeError, call, Symbol·iterator) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@saghul
Copy link
Contributor

saghul commented Dec 18, 2025

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 builtins.js where we put all similar ones? I think I have a soft preference for the single file approach, WDYT?

@bnoordhuis
Copy link
Contributor Author

With a single .js file there's no lazy-loading of individual built-ins; it's all or nothing.

@saghul
Copy link
Contributor

saghul commented Dec 18, 2025

With a single .js file there's no lazy-loading of individual built-ins; it's all or nothing.

Fair enough!

@bnoordhuis
Copy link
Contributor Author

@saghul if you could find it in you to review this pr and the other one?

Copy link
Contributor

@saghul saghul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a suggestion!

@@ -0,0 +1,210 @@
;(function(IteratorHelper, InternalError, TypeError, call, Symbol·iterator) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
;(function(IteratorHelper, InternalError, TypeError, call, Symbol·iterator) {
;(function(IteratorHelper, InternalError, TypeError, call, Symbol.iterator) {

Was that a weird middle dot rather than a period?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see now we are pasing the symbol straight. It mislead me, so may I suggest SymbolIterator ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No strong opinion, land it then!

if (item.done) break
let iter = item.value
check(iter, "bad iterator")
let method = iter[Symbol·iterator]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mark it as TODO because it can be fixed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, or at least, I think it's plausible that it can be fixed.

@bnoordhuis bnoordhuis merged commit 210ecb2 into quickjs-ng:master Jan 7, 2026
122 checks passed
@bnoordhuis bnoordhuis deleted the iterator-zip-take2 branch January 7, 2026 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants