-
Notifications
You must be signed in to change notification settings - Fork 824
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
Added support for JavascriptCore #3825
Conversation
# Conflicts: # examples/wasi.rs # examples/wasi_manual_setup.rs # examples/wasi_pipes.rs # lib/cli/Cargo.toml # lib/wasi/Cargo.toml
…xes-and-pthreads # Conflicts: # lib/c-api/Cargo.toml # lib/cli/Cargo.toml # lib/wasi/Cargo.toml # lib/wasi/src/runtime/mod.rs
@@ -1,5 +1,7 @@ | |||
#[cfg(feature = "js")] | |||
use crate::js::externals::table as table_impl; | |||
#[cfg(feature = "jsc")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we introduce jsc
as a feature, perhaps js
needs to be renamed to jsb
(JavascriptBrowser)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd call it web
.
But that'll need to wait for 4.0
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer not to change current feature flags
@@ -0,0 +1,39 @@ | |||
use std::any::Any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can any of these files be made common between jsc
and js
? Perhaps using a #[path()]
attribute?
// }); | ||
// structuredClone(memory) | ||
// ``` | ||
unsafe impl Send for Memory {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can avoid this it would be better
Ok(()) | ||
} | ||
|
||
pub(crate) fn read_uninit<'b>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this code common between js
and jsc
? It's an area that can easily get bugs so making its footprint smaller will help
/// Set a global, at index idx. Will panic if idx is out of range | ||
/// Safety: the caller should check taht the raw value is compatible | ||
/// with destination VMGlobal type | ||
pub fn set_global_unchecked(&self, idx: usize, new_val: u128) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is not implemented then won't it break threading and forking?
This reverts commit de8cdf7.
@@ -1,5 +1,7 @@ | |||
#[cfg(feature = "js")] | |||
use crate::js::externals::table as table_impl; | |||
#[cfg(feature = "jsc")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd call it web
.
But that'll need to wait for 4.0
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If some of the code could be mutualised with the js backend, that would be great for future maintenance.
Added support for JavascriptCore as a backend in Wasmer, and available through the
jsc
feature in thewasmer
crate