Skip to content

Make Iterator.from spec compliant #689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 15, 2024
Merged

Make Iterator.from spec compliant #689

merged 1 commit into from
Nov 15, 2024

Conversation

saghul
Copy link
Contributor

@saghul saghul commented Nov 14, 2024

Create a proper wrapper around the inner iterator and proxy the calls to next / return.

@saghul saghul requested a review from bnoordhuis November 14, 2024 22:37
@@ -42,14 +42,6 @@ test262/test/built-ins/Date/prototype/setUTCMonth/date-value-read-before-tonumbe
test262/test/built-ins/Date/prototype/setUTCMonth/date-value-read-before-tonumber-when-date-is-invalid.js:26: strict mode: Test262Error: time updated in valueOf Expected SameValue(«NaN», «0») to be true
test262/test/built-ins/Date/prototype/setUTCSeconds/date-value-read-before-tonumber-when-date-is-invalid.js:26: Test262Error: time updated in valueOf Expected SameValue(«NaN», «0») to be true
test262/test/built-ins/Date/prototype/setUTCSeconds/date-value-read-before-tonumber-when-date-is-invalid.js:26: strict mode: Test262Error: time updated in valueOf Expected SameValue(«NaN», «0») to be true
test262/test/built-ins/Iterator/from/get-return-method-when-call-return.js:24: TypeError: not a function
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were the new ones after the test262 update. There are a couple of more failures below in weird property access, but as of this PR Iterator.from is non-ghetto :-P

quickjs.c Outdated
@@ -40053,24 +40057,95 @@ static JSValue js_array_iterator_next(JSContext *ctx, JSValue this_val,
}
}

typedef struct JSIteratorData {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bnoordhuis Let me know what you think of this approach, I couldn't find a way around not having extra stored state.

@saghul saghul mentioned this pull request Nov 15, 2024
@saghul saghul marked this pull request as draft November 15, 2024 08:39
@saghul
Copy link
Contributor Author

saghul commented Nov 15, 2024

Doh, I'm an idiot. Looks like we do need a new class because after fixing the prototype whoes I cannot make test262/test/built-ins/Iterator/from/result-proto.js pass.

New class it is!

@saghul saghul marked this pull request as ready for review November 15, 2024 09:08
@saghul
Copy link
Contributor Author

saghul commented Nov 15, 2024

Updated, PTAL! Note it includes #690

Create a proper wrapper around the inner iterator and proxy the calls to
next / return.
@saghul saghul merged commit 284510f into master Nov 15, 2024
47 checks passed
@saghul saghul deleted the fix-iterator-from branch November 15, 2024 12:35
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