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
sunfishcode
left a comment
There was a problem hiding this comment.
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.
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
stdandcorefeatures 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_stdcompatible". Unfortunately though thischange comes at a cost:
allocandcoreisn't idiomatic. Especially trying todual between types like
HashMapfromstdas well as fromhashbrowncauses imports to be surprising in some cases.no_std, so noneof them actually were. Many crates still imported from
stdordepended 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
stdis necessary to buildwasmtime, andplatforms that natively need to execute
wasmtimewill need to use aRust target that supports
std. Note though that not all ofstdneedsto be supported, but instead much of it could be configured off to
return errors, and
wasmtimewould be configured to gracefully handleerrors.
The goal of this PR is to move
wasmtimeback 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_stdcrates (but fixing the issues mentioned above)or ensuring that the target in Rust has
stdavailable.