Skip to content
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

Proposal: one EXPORTS setting to rule them all #8380

Open
kripken opened this issue Apr 1, 2019 · 21 comments
Open

Proposal: one EXPORTS setting to rule them all #8380

kripken opened this issue Apr 1, 2019 · 21 comments
Assignees

Comments

@kripken
Copy link
Member

kripken commented Apr 1, 2019

Background: Right now to keep alive & export a compiled function one would use EXPORTED_FUNCTIONS, while to ensure a JS library function exists one would use DEFAULT_LIBRARY_FUNCS_TO_INCLUDE, and there is EXPORTED_RUNTIME_METHODS for exporting parts of the JS runtime. This is needlessly complex, and confusing since a C API may be implemented in either compiled code or a JS library, and currently you need to know which in order to properly include and export it.

Proposal:

  • A new EXPORTS setting, which receives a list of things to ensure exist and are exported in the final output. These can be in compiled code, the JS library, or the JS runtime - the user doesn't need to know.
  • Deprecate all the other relevant options (but still support them for backwards compatibility for a long time at least), namely EXPORTED_FUNCTIONS, DEFAULT_LIBRARY_FUNCS_TO_INCLUDE, EXPORTED_RUNTIME_METHODS.
  • No _ prefixing, like EXPORTED_RUNTIME_METHODS etc., and unlike EXPORTED_FUNCTIONS. This moves us further along the path to get us rid of the old prefixing annoyance.

For example, -s EXPORTS=['main', 'emscripten_main_loop', 'AsciiToString'] would export a compiled function, a JS library function, and a runtime function, respectively (and note the lack of _ prefixes).

Thoughts?

cc @juj @sbc100 @jgravelle-google @sbc100 @dschuff

@curiousdannii
Copy link
Contributor

curiousdannii commented Apr 2, 2019

Would the exported functions also lack the _? And if you use EMSCRIPTEN_KEEPALIVE could that also export without the _?

(I think this would be cleaner, but it does make backwards compatibility harder or bloatier.)

@jgravelle-google
Copy link
Contributor

sgtm, from a UX perspective at least

@sbc100
Copy link
Collaborator

sbc100 commented Apr 2, 2019

Sounds good, assuming the distinction isn't needs by any of our users.

@curiousdannii, I share your concern regarding maintaining backwards compat. For this kind of thing I'd like to see an eventual path to completely removal. In stages such as:

  1. Disable the features in STRICT mode, and recommend that devs used STRICT whenever possible to stay up-to-date.
  2. After N months, or N emscripten revisions show a warning when using the deprecated feature.
  3. After another N, turn the warning into a hard error and remove all the supporting code

@dschuff
Copy link
Member

dschuff commented Apr 3, 2019

IIUC it does sort of repurpose the meaning of what an "export" is. We've been thinking of it in the sense of being exported from the compiled module, whereas the others would be actually imported into wasm but "exported" from the Module object. Mostly users don't care about that distinction so this is probably a good idea (although, what about user-written JS library code?). Maybe at least for our own sanity, in code/comments (and maybe documentation) we should have more precise nomenclature, e.g. "Module exports" or something to mean exports from the emscripten package (EXPORTS as proposed here), "wasm exports/imports" to mean exports from the wasm module, and... something else?

@stale
Copy link

stale bot commented Apr 2, 2020

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 7 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale bot added the wontfix label Apr 2, 2020
@curiousdannii
Copy link
Contributor

This still sounds like a good improvement if the details can be worked out.

@stale stale bot removed the wontfix label Apr 2, 2020
@kripken
Copy link
Member Author

kripken commented Apr 6, 2020

@curiousdannii Agreed, yeah. I think this has to wait on removing fastcomp first, after that this will be much easier to do.

@stale
Copy link

stale bot commented Jun 4, 2021

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale bot added the wontfix label Jun 4, 2021
@curiousdannii
Copy link
Contributor

This would still be nice to have.

@sbc100
Copy link
Collaborator

sbc100 commented May 1, 2024

I'm actively work on this now!

@sbc100 sbc100 self-assigned this May 1, 2024
sbc100 added a commit to sbc100/emscripten that referenced this issue May 1, 2024
We only had two symbols in `legacyRuntimeElements` and it seems cleaner
to handle these legacy aliases via the JS library system that we have
for that.

This helps with my work towards emscripten-core#8380
sbc100 added a commit to sbc100/emscripten that referenced this issue May 1, 2024
We only had two symbols in `legacyRuntimeElements` and it seems cleaner
to handle these legacy aliases via the JS library system that we have
for that.

This helps with my work towards emscripten-core#8380
sbc100 added a commit to sbc100/emscripten that referenced this issue May 1, 2024
We only had two symbols in `legacyRuntimeElements` and it seems cleaner
to handle these legacy aliases via the JS library system that we have
for that.

This helps with my work towards emscripten-core#8380
sbc100 added a commit to sbc100/emscripten that referenced this issue May 1, 2024
We only had two symbols in `legacyRuntimeElements` and it seems cleaner
to handle these legacy aliases via the JS library system that we have
for that.

This helps with my work towards emscripten-core#8380
sbc100 added a commit to sbc100/emscripten that referenced this issue May 1, 2024
sbc100 added a commit to sbc100/emscripten that referenced this issue May 1, 2024
sbc100 added a commit that referenced this issue May 1, 2024
We only had two symbols in `legacyRuntimeElements` and it seems cleaner
to handle these legacy aliases via the JS library system that we have
for that.

This helps with my work towards #8380
sbc100 added a commit to sbc100/emscripten that referenced this issue May 1, 2024
sbc100 added a commit that referenced this issue May 2, 2024
@msqr1
Copy link

msqr1 commented May 3, 2024

If possible, please remove the prefix of _ in function exports 👍

@sbc100
Copy link
Collaborator

sbc100 commented May 3, 2024

If possible, please remove the prefix of _ in function exports 👍

Yes, removing the underscore is indeed part of this change. It will go hand in hand.

@msqr1
Copy link

msqr1 commented May 3, 2024

Why were exports prefixed in the first place?

@sbc100
Copy link
Collaborator

sbc100 commented May 3, 2024

Why were exports prefixed in the first place?

I believe is comes from windows and/or macOS where the C ABI includes a leading underscore on all C symbols. I guess whoever worked on this originally was probably a windows and/or macOS user.

See https://stackoverflow.com/questions/1034852/adding-leading-underscores-to-assembly-symbols-with-gcc-on-win32

@dschuff
Copy link
Member

dschuff commented May 3, 2024

I thought it had more to do with the fact that back in the asm.js days there was the potential of naming conflicts between the asm-compiled part and the JS wrapper part...

@sbc100
Copy link
Collaborator

sbc100 commented May 3, 2024

I thought it had more to do with the fact that back in the asm.js days there was the potential of naming conflicts between the asm-compiled part and the JS wrapper part...

Not as far as I know. I don't see how the potential for naming conflict was any different back then. Native and JS symbols still occupy the same name space today. I think it just came from the fact that C compiler symbols on macOS/windows have/had leading underscores. IIUC it just seemed natural for native symbols to be named like that when @kripken and @juj were first working on fastcomp.

@msqr1
Copy link

msqr1 commented May 3, 2024

Well if there was naming conflict, should we just prefix our library function with something like EM_

@sbc100
Copy link
Collaborator

sbc100 commented May 3, 2024

Well if there was naming conflict, should we just prefix our library function with something like EM_

There are two types of JS library symbols:

  1. Symbols that we want to be in the same namespace as C symbols, and which implement C APIs. In this case we want the JS symbol to have the same name as the C symbol. These symbols are directly callable from C. In the JS library files these are the ones that don't have the $ prefix.
  2. Symbols that are not designed to be called from C but are just JS utility function. In the JS library files these are the ones that have the $ prefix. (you can't link your C code against these).

In addition to these JS library symbols we also two other types of symbols in the JS world:

  1. Emscripten runtime symbols, not part of the JS library, and not callable from native code (you can't link your C code against thse).
  2. Builtin JS symbols such as window or document (you can't link your C code against these).

The only time we need to worry about symbol collision is if user explicitly exports a C symbol that happens to have the same name as a symbol of type (2), (3) or (4) above. I don't think this is going to be very common to be honest, and we don't have the ability to rename symbols of type (4) at all, so I'm inclined not to worry about it, and issue warnings/errors to users who try to export colliding symbols.

@kripken
Copy link
Member Author

kripken commented May 6, 2024

Historically the reason we had prefixing had a few reasons that include the ones mentioned. The very first Emscripten translated LLVM IR text into JS, and I needed to handle name collisions between %foo (a local) and @foo (a function name), so % and @ each got translated into something (_ and $). And this seemed natural because various assembly notations have _ prefixes too. So it was both because of name conflicts and convention.

@msqr1
Copy link

msqr1 commented Jun 17, 2024

What is our progress on this? @sbc100

sbc100 added a commit to sbc100/emscripten that referenced this issue Nov 21, 2024
This change adds a new `-sEXPORT` setting that can by used to export
any type of symbols (see emscripten-core#8380)

It also includes a new `-sMANGLED_SYMBOLS` setting which default to
true in order to not break backwards compatibility.

Both these new settings are currently experimental and using `-sEXPORT`
currently disables `-sMANGLED_SYMBOLS` by default.
@sbc100
Copy link
Collaborator

sbc100 commented Nov 21, 2024

I just uploaded the a draft PR: #22977

sbc100 added a commit to sbc100/emscripten that referenced this issue Nov 21, 2024
This change adds a new `-sEXPORT` setting that can by used to export
any type of symbols (see emscripten-core#8380)

It also includes a new `-sMANGLED_SYMBOLS` setting which default to
true in order to not break backwards compatibility.

Both these new settings are currently experimental and using `-sEXPORT`
currently disables `-sMANGLED_SYMBOLS` by default.
sbc100 added a commit to sbc100/emscripten that referenced this issue Nov 21, 2024
This change adds a new `-sEXPORT` setting that can by used to export
any type of symbols (see emscripten-core#8380)

It also includes a new `-sMANGLED_SYMBOLS` setting which default to
true in order to not break backwards compatibility.

Both these new settings are currently experimental and using `-sEXPORT`
currently disables `-sMANGLED_SYMBOLS` by default.
sbc100 added a commit to sbc100/emscripten that referenced this issue Nov 21, 2024
This change adds a new `-sEXPORT` setting that can by used to export
any type of symbols (see emscripten-core#8380)

It also includes a new `-sMANGLED_SYMBOLS` setting which default to
true in order to not break backwards compatibility.

Both these new settings are currently experimental and using `-sEXPORT`
currently disables `-sMANGLED_SYMBOLS` by default.
sbc100 added a commit to sbc100/emscripten that referenced this issue Jan 2, 2025
This change adds a new `-sEXPORT` setting that can by used to export
any type of symbols (see emscripten-core#8380)

It also includes a new `-sMANGLED_SYMBOLS` setting which default to
true in order to not break backwards compatibility.

Both these new settings are currently experimental and using `-sEXPORT`
currently disables `-sMANGLED_SYMBOLS` by default.
sbc100 added a commit to sbc100/emscripten that referenced this issue Jan 2, 2025
This change adds a new `-sEXPORT` setting that can by used to export
any type of symbols (see emscripten-core#8380)

It also includes a new `-sMANGLED_SYMBOLS` setting which default to
true in order to not break backwards compatibility.

Both these new settings are currently experimental and using `-sEXPORT`
currently disables `-sMANGLED_SYMBOLS` by default.
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

No branches or pull requests

6 participants