-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Detect and report late assignment of Module API elements #23629
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
Conversation
602f717
to
7a690c5
Compare
CC @sgbeal |
Thank you for the heads up! i'll keep an eye out for any fallout on our end.
What happens when asserts are not enabled? Does that make such usage "legal but possibly ill-advised" or "undefined behavior"? |
The same thing that happens today: nothing at all. The assignment will go undetected and un-used. |
When we try to detect such API misused, or change the behaviour of emscripten like this, we normally focus on reported these errors only when assertions are enabled, to avoid increasing code size in release builds. |
We had some bug reports recently when we switched to using async/await in MODULUARIZE mode. One of the side effects was the `--post-js` code then runs after module instantiation and startup, which means assignment of properties like `onRuntimeInitialized` is too late if it happens in `--post-js`. This was always the case for sync instantiation too. We now detect this too-late-assignment at abort with a useful error. See emscripten-core#23626 and emscripten-core#23420
7a690c5
to
7e3e35c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea!
We had some bug reports recently when we switched to using async/await
in
MODULUARIZE
mode. One of the side effects was the--post-js
codethen runs after module instantiation and startup, which means assignment
of properties like
onRuntimeInitialized
is too late if it happens in--post-js
.This was always the case for sync instantiation mode.
When assertions are enabled, we now detect this too-late-assignment and abort
with a useful error.
Code size regression in this PR, as expected, only for debug builds.
See #23626 and #23420