Enable EM_JS in side modules #19705
Merged
Enable EM_JS in side modules #19705
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sbc100
force-pushed
the
em_js_side_modules
branch
from
c823051
to
3ae2e6e
Compare
sbc100
force-pushed
the
em_js_side_modules
branch
from
3ae2e6e
to
f9cdf62
Compare
kripken
reviewed
Jun 26, 2023
emcc.py
Outdated
| @@ -831,6 +831,7 @@ def process_dynamic_libs(dylibs, lib_dirs): | |||
| for dylib in dylibs: | |||
| exports = webassembly.get_exports(dylib) | |||
| exports = set(e.name for e in exports) | |||
| exports = [utils.removeprefix(e, '__em_js__') for e in exports] | |||
There was a problem hiding this comment.
Why remove this prefix? A comment might be good as it looks surprising here I think.
Also, it seems like keeping the prefix might avoid the risk of a conflict with something else?
There was a problem hiding this comment.
Added a comment.
The point of this code here is simply to decide which symbols are allowed to be undefined in the main module (i.e. which symbols will be provided by the side module). Duplicates here are not going to be problem, but I'm actually not sure what happens in the normal (non-shared) case when a symbol is exported both as a normal function and as a EM_JS function... whatever the case that would be an existing bug.
sbc100
force-pushed
the
em_js_side_modules
branch
from
f9cdf62
to
825fef4
Compare
sbc100
force-pushed
the
em_js_side_modules
branch
from
825fef4
to
ff5d29b
Compare
This works in a similar way to EM_ASM. See #18228. Depends on WebAssembly/binaryen#5780
sbc100
force-pushed
the
em_js_side_modules
branch
from
ff5d29b
to
b53ba11
Compare
kripken
approved these changes
Jun 26, 2023
sbc100
added a commit
to sbc100/emscripten
that referenced
this pull request
Oct 11, 2023
This was fixed in emscripten-core#19705.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This works in a similar way to EM_ASM. See #18228.
Depends on WebAssembly/binaryen#5780