Skip to content

no_std support in the Rust guest bindings. #345

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

Merged
merged 5 commits into from
Oct 5, 2022

Conversation

sunfishcode
Copy link
Member

This adds no_std support in the Rust guest bindings. Mostly this involves using core and alloc instead of std, but it also involves adding extern crate alloc; in a few places, and also adding a "std" cargo feature to gen-guest-rust so that the impl std::error::Error can be made conditional.

This will eventually be useful for generating bindings from the WASI wit files for std itself to use. And, it's useful for experimenting with generating minimal bindings.

@alexcrichton
Copy link
Member

I think this is reasonable to land for implementing the standard library itself but I'm wary about the "no_std support" moniker since that seems to equate to "I want to compile for embedded things which don't have access to libstd" which doesn't make sense for wasm since libstd is always there.

In terms of organization could most of the references to the alloc trait be pushed into the runtime crate? I'd rather not generate a lot of extern crate alloc; all over the place in the generated code an hygienically referring to alloc through the runtime crate seems beneficial anyway. Otherwise though referencing core::* items seems reasonable to me.

This adds no_std support in the Rust guest bindings. Mostly this
involves using `core` and `alloc` instead of `std`, but it also
involves adding `extern crate alloc;` in a few places, and also
adding a `"std"` cargo feature to gen-guest-rust so that the
`impl std::error::Error` can be made conditional.

This will eventually be useful for generating bindings from the
WASI wit files for std itself to use. And, it's useful for
experimenting with generating minimal bindings.
@sunfishcode
Copy link
Member Author

I think this is reasonable to land for implementing the standard library itself but I'm wary about the "no_std support" moniker since that seems to equate to "I want to compile for embedded things which don't have access to libstd" which doesn't make sense for wasm since libstd is always there.

Right; the use cases I'm looking at are: ways to build highly specialized polyfills that don't work like regular modules, and eventually, building std.

In terms of organization could most of the references to the alloc trait be pushed into the runtime crate? I'd rather not generate a lot of extern crate alloc; all over the place in the generated code an hygienically referring to alloc through the runtime crate seems beneficial anyway. Otherwise though referencing core::* items seems reasonable to me.

I've now factored out all the extern crate alloc;s into the runtime crate.

Comment on lines 15 to 17
[features]
default = ["std"]
std = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading over this again, I think a compile-time option may not be the best way to shoehorn this feature for the code generators. Could the print_typedef_enum method grow a gen_error: bool method perhaps which is threaded through? For the Wasmtime-host generator this would always pass true but for the Rust-guest generator I think it would be best to have this as a configurable Opts option which, in the proc-macro, would be configured based on a feature flag of the proc-macro itself.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would help cut down on the default-features = false traffic as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done; and yeah, it's a lot simpler that way.

Comment on lines 18 to 19
// Re-export things from liballoc for convenient use.
pub use liballoc::{alloc, string, vec};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could these move into mod rt since that's the "internal stuff exported for the macro" module? (and would also avoid the need to rename alloc to liballoc)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Cargo.toml Outdated
wit-bindgen-gen-rust-lib = { path = 'crates/gen-rust-lib', version = '0.2.0' }
wit-bindgen-guest-rust = { path = 'crates/guest-rust', version = '0.2.0' }
wit-bindgen-gen-rust-lib = { path = 'crates/gen-rust-lib', version = '0.2.0', default-features = false }
wit-bindgen-guest-rust = { path = 'crates/guest-rust', version = '0.2.0', default-features = false }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With my suggestion below it will remove the need to say default-features = false on the gen-guest-rust and gen-rust-lib crates, and with those changes is this still necessary?

I'll admit that I do wish that workspace = true, default-features = false would indeed turn off default features but alas. If that's the point of conflict you're running into I think it might be reasonable to, for that particular dependency line, avoid workspace = false

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that change got rid of all the default-features changes in this PR.

@alexcrichton alexcrichton merged commit a4e4f7a into bytecodealliance:main Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants