Make exported __cpp_exception into a global. NFC #24282
Conversation
This still is not quite a generic as I would like so as a followup we can probably make this work for all tags, and perhaps unify the updating of the globals when the exports are available.
| // In static linking, tags are defined within the wasm module and are | ||
| // exported, whereas in dynamic linking, tags are defined in libcore.js in | ||
| // JS code and wasm modules import them. | ||
| $getCppExceptionTag: () => ___cpp_exception, |
There was a problem hiding this comment.
It looks like this splits the code from this file to two places, here and preamble.js. That seems less simple at first glance - are there other benefits?
There was a problem hiding this comment.
The removal of the dependency on wasmExports is the main benefit.
That currently doesn't work in WASM_ESM_INTEGRATION mode. We could make it work I suppose by adding an import * as wasmExports statement.. but it seems less precise that just importing what is used.
In the long run I think we should have a generic way to handle the non-function, non-global exports of a wasm module.
As of today we have special cases in a lot of places for memory, __indirect_function_table and __cpp_exception but I'd like to eliminate those if possible and replace them with generic export handling.
| @@ -924,6 +924,11 @@ function getWasmImports() { | |||
| #endif | |||
| #endif | |||
|
|
|||
| #if hasExportedSymbol('__cpp_exception') && !RELOCATABLE | |||
There was a problem hiding this comment.
Why do we check for not relocatable?
There was a problem hiding this comment.
Because in relocatable mode symbols like this (along with the wasm table and memory) are not exported by the wasm but created in JS and imported by each wasm module.
There was a problem hiding this comment.
In that case wouldn't hasExportedSymbol be false when relocatable?
There was a problem hiding this comment.
Yes we expect hasExportedSymbol to be false in relocatable mode and true for static builds. Here are are checking for static builds we check for hasExportedSymbol to be true.
There was a problem hiding this comment.
But then is
A = hasExportedSymbol('__cpp_exception') && !RELOCATABLE
not equivalent to
B = hasExportedSymbol('__cpp_exception')
? Looking at the two cases,
- If relocatable, then hasExportedSymbol is false,
A = false && false => false, andB = false. - If not relocatable, hasExportedSymbol may be true or false, but
!relocatableis always true, so it does not cause a difference betweenAandB.
So can't we do this?
| #if hasExportedSymbol('__cpp_exception') && !RELOCATABLE | |
| #if hasExportedSymbol('__cpp_exception') |
There was a problem hiding this comment.
Sure you could argue these are redundant. I'm mostly copying the existing logic for __indirect_function_table above.
|
I think I might wait on landing this until I can come up with a more generic solution,.. |
|
This did actually turn out to be necessary for my ESM integration work. |
This still is not quite a generic as I would like so as a followup we can probably make this work for all tags, and perhaps unify the updating of the globals when the exports are available.