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
Fetch api unified into promises , callback and asynchify #21630
Open
impact-maker
wants to merge
42
commits into
emscripten-core:main
Choose a base branch
from
impact-maker:fetchapisol
base: main
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
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 @tlively Just pushed the first draft for the Fetch API. Kindly provide me some feedback so that I can complete the api restructuring along with test suites |
impact-maker
force-pushed
the
fetchapisol
branch
from
March 29, 2024 22:42
7357e51
to
5eaa342
Compare
…ipten-core#21656) This typo was introduced in emscripten-core#21559.
This is the list of files using 80-bit long double: https://github.com/llvm/llvm-project/blob/6009708b4367171ccdbf4b5905cb6a803753fe18/compiler-rt/lib/builtins/CMakeLists.txt#L279-L294 (This file is from LLVM 17.0.6, which is currently our compiler-rt version) We don't have 80-bit long doubles so it looks we can exclude them.
…ipten-core#21649) * Merge two `SYSCALLS_REQUIRE_FILESYSTEM` blocks. NFC * Split out syscall vararg handling into seperate functions. NFC This means we only include these function when they are needed. As part of a seperate change I was having issue with closure compiler when `SYSCALLS.varargs` are used by never assigned to.
Its possible that passing these function directly to Wasm is easier to optimize in the engine.
This updates libcxx and libcxxabi to LLVM 18.1.2. Due to llvm/llvm-project#65534 we need to update both at the same time. Things to note: - Disable hardening mode: LLVM 18.1.2 adds support for "Hardening Modes" to libcxx (https://libcxx.llvm.org/Hardening.html). There are four modes: none, fast, extensive and debug. Different hardening modes make different trade-offs between the amount of checking and runtime performance. We for now disable it (i.e., set it to 'none') so that we don't have any effects on the performance. We can consider enabling it when we ever get to enable the debug version of libcxx. - Disable C++20 time zone support: LLVM 18.1.2 adds C++20 time zone support (https://libcxx.llvm.org/DesignDocs/TimeZone.html) to libcxx, which requires access IANA Time Zone Database. Currently it seems it only supports Linux: https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/src/tz.cpp#L45-L49 So this excludes the two source files from build (which is done via `CMakeLists.txt` in the upstream LLVM) and sets `_LIBCPP_HAS_NO_TIME_ZONE_DATABASE` macro in `__config_site`. In future maybe we can consider implementing this in JS. - `__cxa_init_primary_exception` support: llvm/llvm-project#65534 introduces `__cxa_init_primary_exception` and uses this in libcxx. Like several other methods like the below in `cxa_exception.cpp`, https://github.com/emscripten-core/emscripten/blob/fbdd9249e939660cb0d20f00d6bc1897c2f3905e/system/lib/libcxxabi/src/cxa_exception.cpp#L264-L269 this new function takes a pointer to a destructor, and in Wasm destructor returns a `this` pointer, which requires `ifdef __USING_WASM_EXCEPTION__` on its signature. And that it is also used in libcxx means we need to guard some code in libcxx with `ifdef __USING_WASM_EXCEPTION__` for the signature difference, and we need to build libcxx with `-D__USING_WASM_EXCEPTIONS__` when Wasm EH is used. Also we add Emscripte EH's counterpart in `cxa_exception_emscripten.cpp` too. - This version fixes long-running misbehaviors of `operator new` llvm/llvm-project#69498 seems to fix some long-running misbhaviors of `operator new`, which in emscripten we tried to fix in emscripten-core#11079 and emscripten-core#20154. So while resolving the conflicts, I effectively reverted emscripten-core#11079 and emscripten-core#20154 in `libcxx/src/stdlib_new_delete.cpp` and `libcxx/src/new.cpp`. - Add default `__assertion_handler` to `libcxx/include`: llvm/llvm-project#77883 adds a way for vendors to override how we handle assertions. If a vendor doesn't want to override it, CMake will copy the provided [default template assertion handler file](https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/vendor/llvm/default_assertion_handler.in) to libcxx's generated include dir, which defaults to `SYSROOT/include/c++/v1`: https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/CMakeLists.txt#L72-L73 https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/CMakeLists.txt#L439 https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/include/CMakeLists.txt#L1024 We don't use CMake, so this renames the provided `vendor/llvm/default_assertion_handler.in` to `__assert_handler` in our `libcxx/include` directory, which will be copied to `SYSROOT/include/c++/v1`. - Disable `__libcpp_verbose_abort directly` in code: In emscripten-core#20707 we decided to disable `__libcpp_verbose_abort` not to incur the code size increase that brings (it brings in `vfprintf` and its family). We disabled it in adding `#define _LIBCPP_AVAILABILITY_HAS_NO_VERBOSE_ABORT` in `__config_site`. But that `_NO_` macros are gone in LLVM 18.1.2, and changing it to `#define _LIBCPP_AVAILABILITY_HAS_VERBOSE_ABORT 0` does not work because it is overridden by this line, unless we provide our own vendor annotations: https://github.com/llvm/llvm-project/blob/38f5596feda3276a8aa64fc14e074334017088ca/libcxx/include/__availability#L138 I asked about this in llvm/llvm-project#87012 and llvm/llvm-project#71002 (comment) (and more comments below) but didn't get an actionable answer yet. So this disables `__libcpp_verbose_abort` in the code directly for now, hopefully temporarily. Each fix is described in its own commit, except for the fixes required to resolve conflicts when merging our changes, which I wasn't able to commit separately. Fixes emscripten-core#21603.
- Update the regex's according to the TODO - Remove checks for f32->f64 legalization since that was removed from binaryen back in https://github.com/WebAssembly/binaryen/pull/#2052 - Allow LEGALIZE_JS_API to work even when in standalone mode (standalone mode is implied by this test since it uses `-o a.out.wasm`. - `e_i64_i64` was removed since it was checking for the internal function which takes an i64 and this function exists, even when the export itself is a legalizes stub. Since we check the truthyness or falsyness of `e_i64_i32` in all cases anyway I think this was redundant.
Also apply some reformatting to some.
This setting can cause issues if used on its own without `-sWASM_BIGINT`. See emscripten-core#21689.
…mscripten-core#21700) This test seems to needless import itself which leads to some strange behaviour where new workers would load script via `importSripts` and then also via `require`.
I had a runtime error in another PR I'm working on where one of the imports was undefined. Accessing this `sig` property should inside the `typeof` check. Also since its only really ever accessed ones (aside from the assertion) I decided to just remove the extra variable.
…#21682) This change doesn't fix emscripten-core#21667 but just encodes the current behavior in a test. As a followup we could consider resolving shared libraries dependencies based on previous command line arguments which would solve emscripten-core#21667 without the need for adding `-L.`.
.s files cannot be sanitized.
This updates compiler-rt to LLVM 18.1.2. Things to note: - llvm/llvm-project#73573 reformatted https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp, so our diff got somewhat bigger because adding a preprocessor like `ifdef` somewhere means changing the indentation of all inner directives. - In https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp, ```diff - #elif SANITIZER_USE_GETAUXVAL - return getauxval(AT_PAGESZ); ``` These lines were somehow deleted in emscripten-core#11662 for no apparent reason. (It looks this is not used by us) Restored it to reduce the diff between us and the upstream. - Build libsanitizer_common_rt with `-Wno-format`: emscripten-core@67108b2 Some code that got newly compiled in this version has violations to `__attribute__ ((format))` that crashes compilation without `-Wno-format`. Detailed description of the problem is in emscripten-core@67108b2. The simple temporary fix of adding `-Wno-format` is basically what the upstream code currently does with `CMakeLists.txt`. - `ThreadStart`-related changes in https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/lib/asan/asan_emscripten.cpp: emscripten-core@36ce3d3 This deals with the following upstream changes: llvm/llvm-project@4e1b55a llvm/llvm-project@fd16d46 by adding `ThreadStartParams` class as https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/asan/asan_win.cpp did. Each fix is described in its own commit, except for the fixes required to resolve conflicts when merging our changes, which I wasn't able to commit separately.
…1719) Lets see if this flakieness still persists. I ran the test 10 times locally and didn't see it.
And also using `!=` when we know the LHS are RHS are both string, specifically `typeof xx` on the left and a string literal on the right.
…core#21637) As pointed out by emscripten-core#21625, when using unique_ptr's with primitive types the raw pointer was being passed between wasm and JS, but it was using the underlying type (e.g. char, int, etc) to convert the value. This means the pointer was truncated to a char at the boundary. Embind doesn't allow pointers or references to primitive types so it doesn't make sense to allow a unique_ptr for these types either.
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.24.9 to 3.24.10. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@1b1aada...4355270) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ile (emscripten-core#21721) Its useful for pre-js files to be able to depend on these variables, and this is specifically necessary for emscripten-core#21701 which removes the default export of `ENVIRONMENT_IS_PTHREAD`. Note that this also fixes an inconsistency between the regular runtime and the minimal runtime where minimal runtime was defining some `ENVIRONMENT_IS_XX` var before pre-js but the regular runtime was not.
Update the expected output of `test_eval_ctors_debug_output` to account for changed debug output due to WebAssembly/binaryen#6464. The test will now succeed before and after that upstream Binaryen change.
) This setting affects whether the ASYNCIFY_ADD list propagates, that is, whether it automatically leads to more things added based on the inference. It is on by default, which is safer, but may increase code size in some cases - to undo that, set it to 0. Fixes emscripten-core#13150
I noticed when working on emscripten-core#21701 then these tests were not renaming the `.js` output, which meant that if `mainScriptUrlOrBlob` was completely ignored the tests would still pass. These changes rename the output `.js` file such that the default name will not longer work and the tests would fail if `mainScriptUrlOrBlob` were ignored.
…-core#21694) Makes the TypeScript output easier to read. Fixes emscripten-core#21642
…core#21734) Outside of `MODULARIZE` this is just a normal variable that can/should be minified.
Last usage looks like it was removed in emscripten-core#7667.
…1736) This always seems to refer to full filename/url. Perhaps in the past it actually did hold the directory name?
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.
Solves #21440