Skip to content

[ASYNCIFY] Add some assertions. NFC #24332

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
May 20, 2025

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented May 15, 2025

No description provided.

@sbc100 sbc100 force-pushed the asyncify_assertion branch from 2657eb3 to b5d8bc9 Compare May 15, 2025 00:25
@sbc100 sbc100 requested a review from kripken May 15, 2025 00:30
@sbc100
Copy link
Collaborator Author

sbc100 commented May 15, 2025

@kripken what do you think about the assertion firing in core0.test_asyncify_side_module_reversed: Aborted(Assertion failed: export not found: main) .. is that OK? Should we fail there? Should we fix the test somehow?

@sbc100 sbc100 force-pushed the asyncify_assertion branch from b5d8bc9 to 56627e0 Compare May 15, 2025 15:43
@kripken
Copy link
Member

kripken commented May 15, 2025

Hmm, that sounds like a bug... why is it trying to create a rewind function for a non-existing export? I'd expect it to create them when it actually needs them.

Perhaps somewhere in the code has a hardcoded assumption about main always existing?

@sbc100 sbc100 force-pushed the asyncify_assertion branch from 56627e0 to 02d1d28 Compare May 19, 2025 23:18
@sbc100
Copy link
Collaborator Author

sbc100 commented May 19, 2025

Ah, I had the assertion in slightly the wrong place. All good now.

@sbc100 sbc100 enabled auto-merge (squash) May 19, 2025 23:18
@sbc100 sbc100 disabled auto-merge May 20, 2025 00:10
@sbc100 sbc100 merged commit 2c3c493 into emscripten-core:main May 20, 2025
25 of 30 checks passed
@sbc100 sbc100 deleted the asyncify_assertion branch May 20, 2025 00:10
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