Skip to content

Add a safe wrapper around ForOfIterator#736

Open
simonwuelker wants to merge 2 commits intoservo:mainfrom
simonwuelker:for-of-iterator
Open

Add a safe wrapper around ForOfIterator#736
simonwuelker wants to merge 2 commits intoservo:mainfrom
simonwuelker:for-of-iterator

Conversation

@simonwuelker
Copy link
Copy Markdown
Member

@simonwuelker simonwuelker commented Apr 19, 2026

ForOfIterator implements for .. of iteration in spidermonkey. It holds stack roots and is a pain to use directly due to some weird shenanigans where bindgen adds extra padding to the struct for some LLVM versions. Its also hard to use in a crash-proof manner, because ForOfIterator must not be moved. We'd have to Pin<Box<>> it to be safe, which adds overhead.

This change instead adds mozjs::rust::for_of, a function that takes a callback and uses ForOfIterator correctly so users don't have to worry about it.

Testing: The iterator is used during conversion of IDL sequences, and its covered by WPT. I'm also using it for web animations locally.
Part of servo/servo#36950
Servo PR: servo/servo#44356

Comment thread mozjs-sys/build.rs Outdated
@simonwuelker
Copy link
Copy Markdown
Member Author

Hmm need to investigate why this crashes in the vec_conversion test on windows.

@simonwuelker simonwuelker marked this pull request as draft April 20, 2026 08:13
@simonwuelker simonwuelker changed the title Make ForOfIterator more convenient to use Add a safe wrapper around ForOfIterator Apr 28, 2026
@simonwuelker simonwuelker force-pushed the for-of-iterator branch 3 times, most recently from ccf0c8c to ed4c4cb Compare April 28, 2026 13:05
@simonwuelker simonwuelker marked this pull request as ready for review April 28, 2026 13:27
@simonwuelker
Copy link
Copy Markdown
Member Author

This is ready for review now. Some jobs are not picked up by the runners, not sure what's up with that.

@sagudev
Copy link
Copy Markdown
Member

sagudev commented Apr 28, 2026

Some jobs are not picked up by the runners, not sure what's up with that.

We are limited by number of concurrent mac runners (org wise), so it usually takes some time for them to get running. That is also way I canceled old CI runs, as they result will be thrown away anyway.

@sagudev sagudev self-requested a review April 28, 2026 13:38
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
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