Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented May 15, 2025

Also enable a bunch more tests

@sbc100 sbc100 force-pushed the modularize_instance_tests branch from 41d5bee to 690ca19 Compare May 15, 2025 18:01
@sbc100 sbc100 changed the title [MODULARIZE=instance] Fix more tests in this mode. NFC [MODULARIZE=instance] Mark -sDYNCALLS as not supported. NFC May 15, 2025
@sbc100 sbc100 requested a review from brendandahl May 15, 2025 18:01
@sbc100 sbc100 force-pushed the modularize_instance_tests branch 2 times, most recently from 33b19f4 to 4bf37f7 Compare May 15, 2025 18:20
@sbc100 sbc100 requested a review from dschuff May 15, 2025 18:21
@sbc100 sbc100 force-pushed the modularize_instance_tests branch 5 times, most recently from 242e328 to ff802f5 Compare May 15, 2025 20:09
#if PTHREADS
// When run as a pthread we run `init` immediately.
if (ENVIRONMENT_IS_PTHREAD) await init()
#if PTHREADS || WASM_WORKERS
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this suppose to be in this PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes without this the test_embind_wasm_workers doesn't work. This basically makes wasm worker work in this mode.


def test_embind_val_coro(self):
create_file('post.js', r'''Module.onRuntimeInitialized = () => {
create_file('pre.js', r'''Module.onRuntimeInitialized = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the conversion from post.js to pre.js here and below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding onRuntimeInitialized during post.js doesn't work in the case we do top-level-await on the wasm module because in that case (and also in the case of sync instantiation) the initialization has already been done before the post-js stuff runs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This changes fixes this test so that it works in all mode.

// Weak map of handle functions to their wrapper. Used to implement
// addEventListener/removeEventListener.
var wrappedHandlers = new WeakMap();
globalThis.onmessage = null;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I could split the wasm workers stuff into a separate PR but I've already done a lot of splitting. Let me know if you want me to split this one off too.

@sbc100 sbc100 force-pushed the modularize_instance_tests branch 2 times, most recently from a1aab55 to e3ecbf4 Compare May 15, 2025 20:32
Also by extension `-sASYNCIFY=1` is not supported either.

Also enable a bunch more tests
@sbc100 sbc100 force-pushed the modularize_instance_tests branch from e3ecbf4 to 86ff51e Compare May 15, 2025 20:34
@sbc100 sbc100 requested a review from brendandahl May 15, 2025 20:34
@sbc100 sbc100 merged commit 7f2c9bf into emscripten-core:main May 15, 2025
17 of 28 checks passed
@sbc100 sbc100 deleted the modularize_instance_tests branch May 15, 2025 21:02
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.

3 participants