-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Migrate back to std::
stylistically
#554
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
Migrate back to std::
stylistically
#554
Conversation
This commit performs a similar transformation as bytecodealliance/wasmtime#554, with similar motivation listed out there as well.
c72cf28
to
4049087
Compare
Ok I've rebased this on the current master branch, and @sunfishcode want to look at the documentation I've added as well? I've opted to fill out this page and I've put down some basic words about what's currently supported and then also filled out how |
This commit moves away from idioms such as `alloc::` and `core::` as imports of standard data structures and types. Instead it migrates all crates to uniformly use `std::` for importing standard data structures and types. This also removes the `std` and `core` features from all crates to and removes any conditional checking for `feature = "std"` All of this support was previously added in bytecodealliance#407 in an effort to make wasmtime/cranelift "`no_std` compatible". Unfortunately though this change comes at a cost: * The usage of `alloc` and `core` isn't idiomatic. Especially trying to dual between types like `HashMap` from `std` as well as from `hashbrown` causes imports to be surprising in some cases. * Unfortunately there was no CI check that crates were `no_std`, so none of them actually were. Many crates still imported from `std` or depended on crates that used `std`. It's important to note, however, that **this does not mean that wasmtime will not run in embedded environments**. The style of the code today and idioms aren't ready in Rust to support this degree of multiplexing and makes it somewhat difficult to keep up with the style of `wasmtime`. Instead it's intended that embedded runtime support will be added as necessary. Currently only `std` is necessary to build `wasmtime`, and platforms that natively need to execute `wasmtime` will need to use a Rust target that supports `std`. Note though that not all of `std` needs to be supported, but instead much of it could be configured off to return errors, and `wasmtime` would be configured to gracefully handle errors. The goal of this PR is to move `wasmtime` back to idiomatic usage of features/`std`/imports/etc and help development in the short-term. Long-term when platform concerns arise (if any) they can be addressed by moving back to `no_std` crates (but fixing the issues mentioned above) or ensuring that the target in Rust has `std` available.
4049087
to
9e6c9a5
Compare
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.
This looks good to me.
As additional background for folks following along, I wrote many of the no_std
changes that the PR here removes. Independently of this PR, I've been thinking for a while now that it'd be nice if we could factor out all our usual no_std
tricks into a helper crate, so that we don't need quite so many changes throughout the tree. Just recently, I discovered there's already a no-std-compat crate which does exactly that.
So, converting the bulk of the casebase back to the std
style, as this PR does, feels like the right thing to do in any case. If do we reintroduce no_std
support (and as the docs in the PR say, please talk to us if you're interested!), I expect we'd redo it anyway, in terms of something like no-std-compat
so that it's much less invasive.
|
||
* Linux x86\_64 | ||
* macOS x86\_64 | ||
* Windows x86\_64 |
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'm not sure if it's worth mentioning here, but at the moment there are a handful of WASI tests which aren't currently supported on Windows yet, though we are working on addressing these.
This commit performs a similar transformation as bytecodealliance/wasmtime#554, with similar motivation listed out there as well.
This commit performs a similar transformation as bytecodealliance/wasmtime#554, with similar motivation listed out there as well.
This commit moves away from idioms such as
alloc::
andcore::
asimports of standard data structures and types. Instead it migrates all
crates to uniformly use
std::
for importing standard data structuresand types. This also removes the
std
andcore
features from allcrates to and removes any conditional checking for
feature = "std"
All of this support was previously added in #407 in an effort to make
wasmtime/cranelift "
no_std
compatible". Unfortunately though thischange comes at a cost:
alloc
andcore
isn't idiomatic. Especially trying todual between types like
HashMap
fromstd
as well as fromhashbrown
causes imports to be surprising in some cases.no_std
, so noneof them actually were. Many crates still imported from
std
ordepended on crates that used
std
.It's important to note, however, that this does not mean that wasmtime
will not run in embedded environments. The style of the code today and
idioms aren't ready in Rust to support this degree of multiplexing and
makes it somewhat difficult to keep up with the style of
wasmtime
.Instead it's intended that embedded runtime support will be added as
necessary. Currently only
std
is necessary to buildwasmtime
, andplatforms that natively need to execute
wasmtime
will need to use aRust target that supports
std
. Note though that not all ofstd
needsto be supported, but instead much of it could be configured off to
return errors, and
wasmtime
would be configured to gracefully handleerrors.
The goal of this PR is to move
wasmtime
back to idiomatic usage offeatures/
std
/imports/etc and help development in the short-term.Long-term when platform concerns arise (if any) they can be addressed by
moving back to
no_std
crates (but fixing the issues mentioned above)or ensuring that the target in Rust has
std
available.