Conversation
Emscripten 4.0.12 introduced breaking change by changing module factory to always return promise even if `WASM_ASYNC_COMPILATION=0`. Due to the last def, it's necessary to strip `await` keyword from generated `resolc.js` to keep it synchronous for C FFI.
| // 2. Remove all "await" keywords. | ||
| // With WASM_ASYNC_COMPILATION=0 all awaited values are not Promises, | ||
| // so removing await just evaluates the expression directly. |
There was a problem hiding this comment.
Can't there be other operations not affected by WASM_ASYNC_COMPILATION that still need to be awaited?
There was a problem hiding this comment.
With WASM_ASYNC_COMPILATION=0 (which we use) they should be evaluated directly, so it should be safe to remove them
There was a problem hiding this comment.
Then wouldn't it essentially be the same as skipping this step altogether? Then it'd be less modifications and less risk of potentially removing a needed await somewhere else.
There was a problem hiding this comment.
Unfortunately, it's necessary to remove them, otherwise there will be syntax error.
| console.log('Changed "return readyPromise" to "return Module"'); | ||
| } | ||
|
|
||
| if (!modified) { |
There was a problem hiding this comment.
Currently, the script succeeds if modified was updated one or more times. Should it succeed even if it was only updated once? Or perhaps some combination thereof.
There was a problem hiding this comment.
interesting question. The resolc.js file is generated as a part of llvm release, so it always has same structure. Assuming bump of emscripten won't happen in nearest future, even updating and releasing new LLVM binary won't change resolc.js content, therefore it should be fine to just keep a check if (!modified) error
Writing more generic check, i.e. modified != 3 might be useful if we find emscripten will constantly change content of the generated JS file.
That said, I think it might be an overkill now, but can be useful if there will be issues with that script
There was a problem hiding this comment.
Alright 👍 (Yeah, was thinking that if e.g. "return async function(moduleArg" was replaced, should we then always expect one (or some) of the other steps to also have replaced something in particular. If so, then we could detect early if the something went wrong in the script.)
There was a problem hiding this comment.
Does usage change for the end-user?
There was a problem hiding this comment.
It should not. Do you think it can increase compile time?
|
@smiasojed deferring the review to you since I'm afk |
3 participants
Emscripten 4.0.12 introduced breaking change by changing module factory to always return promise even if
WASM_ASYNC_COMPILATION=0. Due to the last def, it's necessary to stripawaitkeyword from generatedresolc.jsto keep it synchronous for C FFI.